Conversation
|
The latest updates on your projects. Learn more about Vercel for GitHub.
|
|
Note Reviews pausedIt looks like this branch is under active development. To avoid overwhelming you with review comments due to an influx of new commits, CodeRabbit has automatically paused this review. You can configure this behavior by changing the Use the following commands to manage reviews:
Use the checkboxes below for quick actions:
📝 WalkthroughWalkthroughAdds ~20 Next.js API routes under app/api/research/*, ~40 research helpers and MCP tool registrations in lib/research and lib/mcp/tools/research, Chartmetric/Parallel/Exa client utilities, a Chartmetric token helper, and a new content template entry "album-record-store". Routes handle CORS and delegate logic to new lib handlers. Changes
Sequence Diagram(s)sequenceDiagram
rect rgba(220,220,255,0.5)
participant Client
participant Route as "API Route"
participant Handler as "Research Handler"
end
rect rgba(220,255,220,0.5)
participant Auth as "validateAuthContext"
participant Credits as "deductCredits"
end
rect rgba(255,220,220,0.5)
participant Chartmetric
participant Token as "getChartmetricToken"
end
Client->>Route: GET /api/research/{endpoint}?artist=...
Route->>Handler: delegate request to getResearch*Handler(request)
Handler->>Auth: validateAuthContext(request)
Auth-->>Handler: accountId or NextResponse(401)
alt authenticated
Handler->>Credits: deductCredits(accountId, cost)
Credits-->>Handler: success or throw
Handler->>Token: getChartmetricToken()
Token-->>Handler: access_token
Handler->>Chartmetric: GET /{built-path}?{params} (Bearer token)
Chartmetric-->>Handler: { status, data }
Handler-->>Route: NextResponse(200, payload) + CORS
else unauthenticated
Auth-->>Route: NextResponse(401) + CORS
end
Route-->>Client: HTTP response + CORS
sequenceDiagram
participant Client
participant MCP as "MCP Server"
participant Register as "registerAllResearchTools"
participant Tool as "research_* Tool"
participant Resolve as "resolveArtist"
participant Proxy as "proxyToChartmetric"
MCP->>Register: registerAllResearchTools(server)
Register->>Tool: server.registerTool('research_artist'...)
Client->>MCP: invoke research_artist({ artist })
MCP->>Tool: Tool.handler(args)
Tool->>Resolve: resolveArtist(args.artist)
Resolve->>Proxy: GET /search?q=...&type=artists
Proxy-->>Resolve: { artists: [...] }
Tool->>Proxy: GET /artist/{id}/...
Proxy-->>Tool: { data }
Tool-->>MCP: getToolResultSuccess(data)
MCP-->>Client: tool result
Estimated code review effort🎯 4 (Complex) | ⏱️ ~45 minutes Possibly related PRs
Suggested reviewers
Poem
🚥 Pre-merge checks | ❌ 1❌ Failed checks (1 warning)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing Touches🧪 Generate unit tests (beta)
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
There was a problem hiding this comment.
Actionable comments posted: 9
🧹 Nitpick comments (31)
app/api/research/career/route.ts (1)
5-7: Upgrade placeholder JSDoc to actual API documentation.Line 5-Line 7 and Line 12-Line 15 should describe endpoint behavior and request expectations instead of empty placeholders.
As per coding guidelines, "
app/api/**/route.ts: All API routes should have JSDoc comments".Also applies to: 12-15
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@app/api/research/career/route.ts` around lines 5 - 7, Replace the empty JSDoc blocks with real API documentation describing the route behavior and request expectations: document the exported route handler functions (e.g., GET/POST handlers in this file), the endpoint purpose, accepted request parameters/body shape, expected response shape and status codes, and any authorization or error cases; update both JSDoc blocks (lines around the top of the file and the later block) so they clearly state input/output contracts and side effects to satisfy the "app/api/**/route.ts: All API routes should have JSDoc comments" guideline.app/api/research/curator/route.ts (2)
1-18: Consider a shared route-wrapper utility for research endpoints.Line 8-Line 10 and Line 16-Line 17 follow the same passthrough pattern repeated across these new research routes. A small factory/helper would reduce repetition and keep future endpoint additions safer.
As per coding guidelines, "
**/*.{ts,tsx}: Extract shared logic into reusable utilities following Don't Repeat Yourself (DRY) principle".🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@app/api/research/curator/route.ts` around lines 1 - 18, The OPTIONS and GET handlers in this file duplicate passthrough logic (calling getCorsHeaders and delegating to getResearchCuratorHandler); factor this pattern into a reusable route wrapper (e.g., a utility that builds standard handlers for research endpoints) and replace the explicit OPTIONS and GET implementations with calls to that wrapper; reference the existing symbols OPTIONS, GET, getCorsHeaders and getResearchCuratorHandler when implementing the factory so it returns a standard OPTIONS response with CORS headers and a GET handler that delegates to the provided handler function.
5-7: Replace placeholder JSDoc with endpoint-level contract details.The blocks at Line 5-Line 7 and Line 12-Line 15 are empty placeholders. Please document method intent, required query params, and key error cases.
Proposed doc-only update
-/** - * - */ +/** + * OPTIONS /api/research/curator + * Handles CORS preflight for the curator research endpoint. + */ export async function OPTIONS() { return new NextResponse(null, { status: 200, headers: getCorsHeaders() }); } -/** - * - * `@param` request - */ +/** + * GET /api/research/curator + * Returns curator research data for the provided query parameters. + * + * `@param` request Incoming request with query params. + */ export async function GET(request: NextRequest) { return getResearchCuratorHandler(request); }As per coding guidelines, "
app/api/**/route.ts: All API routes should have JSDoc comments".Also applies to: 12-15
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@app/api/research/curator/route.ts` around lines 5 - 7, The file has empty JSDoc placeholders for the API endpoint; replace each placeholder block with an endpoint-level contract: a short description of intent, the HTTP method(s) handled (e.g., GET/POST), required query parameters or body schema (names and types), expected success response shape and status code, and key error cases with their status codes (validation, auth, not-found, server errors). Update the JSDoc immediately above the route handler functions (the exported GET/POST handlers in app/api/research/curator/route.ts) so it documents required params, example requests/responses, and any thrown errors.app/api/research/lookup/route.ts (1)
5-7: Add substantive JSDoc for route contract clarity.At Line 5-Line 7 and Line 12-Line 15, please replace empty placeholders with method intent, required params, and key error responses.
As per coding guidelines, "
app/api/**/route.ts: All API routes should have JSDoc comments".Also applies to: 12-15
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@app/api/research/lookup/route.ts` around lines 5 - 7, Add substantive JSDoc above the exported route handler functions (e.g., the exported async GET/POST handlers in this file) replacing the empty blocks: state the HTTP method and high-level intent, list required query or body parameters (names, types, whether optional), describe the successful response shape and status code (e.g., 200 with JSON { ... }), and enumerate key error responses and their status codes (e.g., 400 for validation errors, 401/403 for auth, 404 if no resource, 500 for server errors). Include any auth/permission requirements and examples of expected input/output where helpful so the route contract is clear to callers and maintainers.lib/research/getResearchProfileHandler.ts (1)
7-9: Prefer provider-agnostic wording in endpoint docs.Line 7 references “Chartmetric artist profile.” Since this API layer is intended to stay provider-agnostic, consider generic wording in the handler JSDoc.
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@lib/research/getResearchProfileHandler.ts` around lines 7 - 9, Update the JSDoc for getResearchProfileHandler to remove provider-specific wording (remove "Chartmetric") and use generic phrasing like "Returns the full artist profile" or "Returns the full research artist profile" while keeping the note about requiring the `artist` query param (name, numeric ID, or UUID); locate the JSDoc comment above the getResearchProfileHandler function and replace the provider-specific sentence with the provider-agnostic wording.app/api/research/cities/route.ts (1)
5-7: Provide meaningful JSDoc for both handlers.At Line 5-Line 7 and Line 12-Line 15, the comments are placeholders; add concrete method/param/response documentation.
Based on learnings, "Applies to app/api/**/route.ts : All API routes should have JSDoc comments".
Also applies to: 12-15
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@app/api/research/cities/route.ts` around lines 5 - 7, Replace the placeholder comment blocks with meaningful JSDoc for each exported route handler: add a JSDoc above export async function GET describing the endpoint purpose, accepted query params, the shape of the returned JSON and possible status codes; add a JSDoc above export async function POST describing the request body schema, validation rules, created resource format, status codes (e.g., 201/400/500), and any auth/permission requirements or thrown errors; ensure each JSDoc includes `@param` for request/query/body, `@returns` for response shape and status, and `@throws` (or `@throws`) for error cases so the handlers are fully documented.app/api/research/similar/route.ts (1)
5-7: Replace placeholder comments with concrete JSDoc.Line 5-Line 7 and Line 12-Line 15 should document endpoint behavior and required query semantics.
As per coding guidelines, "
app/api/**/route.ts: All API routes should have JSDoc comments".Also applies to: 12-15
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@app/api/research/similar/route.ts` around lines 5 - 7, The file contains placeholder block comments; replace them with concrete JSDoc for the API route: add a top-level JSDoc that describes the endpoint purpose (what "similar" returns), accepted HTTP method(s) and example usage, and for the exported GET handler (or any exported function named GET) add JSDoc describing required query parameters (names, types, whether required), expected response shape and status codes, and any error conditions; ensure the JSDoc explicitly documents required query semantics (e.g., "q: string, required", "limit: number, optional, default 10") and update both comment blocks referenced (the header comment and the block above the GET handler) accordingly.app/api/research/discover/route.ts (1)
5-7: Fill in the JSDoc placeholders with real route docs.Line 5-Line 7 and Line 12-Line 15 currently provide no contract detail. Please include endpoint purpose, required query params, and expected error statuses.
Based on learnings, "Applies to app/api/**/route.ts : All API routes should have JSDoc comments".
Also applies to: 12-15
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@app/api/research/discover/route.ts` around lines 5 - 7, Replace the empty JSDoc blocks with a real route contract for the exported route handler (e.g., the GET handler or exported default function in this file): document the endpoint purpose (what "discover" returns), list required and optional query parameters by name and type (e.g., q: string, page: number, limit: number), describe the successful response shape and status code (200 + JSON schema), and enumerate expected error statuses and conditions (400 for bad request/missing params, 401 for unauthorized, 500 for server errors) including example request and response snippets; ensure the JSDoc tags `@route`, `@param`, `@returns`, and `@throws/`@errors are present and match the handler function name (GET or default export) so consumers know the contract.app/api/research/route.ts (1)
8-10: Consider centralizing preflight response creation to reduce repeated route boilerplate.
OPTIONS()is duplicated across the new research routes; extracting a tiny helper/factory would reduce maintenance overhead and keep behavior uniform.As per coding guidelines:
**/*.{ts,tsx}: Extract shared logic into reusable utilities following Don't Repeat Yourself (DRY) principle.🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@app/api/research/route.ts` around lines 8 - 10, Extract the duplicated preflight response creation into a reusable helper (e.g., preflightResponse or createPreflightResponse) that returns new NextResponse(null, { status: 200, headers: getCorsHeaders() }) and replace each route's OPTIONS() implementation with a single call to that helper; ensure the helper imports/uses getCorsHeaders and NextResponse so all research routes call the same function for uniform CORS preflight handling.app/api/research/profile/route.ts (1)
5-7: Document this route with meaningful JSDoc (not placeholder blocks).Lines 5-7 and Lines 12-15 should describe preflight behavior and the GET contract so the route is self-explanatory.
📝 Proposed doc update
/** - * + * OPTIONS /api/research/profile + * + * Handles CORS preflight for the profile research endpoint. */ export async function OPTIONS() { return new NextResponse(null, { status: 200, headers: getCorsHeaders() }); } /** - * + * GET /api/research/profile + * + * Returns profile-level research data for the requested artist. + * * `@param` request */ export async function GET(request: NextRequest) { return getResearchProfileHandler(request); }Based on learnings: Applies to app/api/**/route.ts : All API routes should have JSDoc comments.
Also applies to: 12-15
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@app/api/research/profile/route.ts` around lines 5 - 7, Replace the placeholder JSDoc in app/api/research/profile/route.ts with a concise, meaningful comment that documents preflight behavior and the GET contract: state that the route responds to OPTIONS preflight with appropriate CORS headers (if applicable) and that the exported GET function accepts any query or auth requirements (e.g., required headers, session) and returns JSON describing the user profile, list the expected response shape (fields and types), possible status codes (200, 401, 400, 500) and error conditions, and note content-type and any caching or rate-limit behavior; reference the exported GET handler name and any helper functions used so a reader can find the implementation quickly.app/api/research/playlist/route.ts (1)
5-7: Replace empty JSDoc blocks with endpoint-specific descriptions.Lines 5-7 and Lines 12-15 currently don’t document behavior, params, or intent.
📝 Proposed doc update
/** - * + * OPTIONS /api/research/playlist + * + * Handles CORS preflight for the playlist research endpoint. */ export async function OPTIONS() { return new NextResponse(null, { status: 200, headers: getCorsHeaders() }); } /** - * + * GET /api/research/playlist + * + * Returns playlist-level research data for the requested playlist context. + * * `@param` request */ export async function GET(request: NextRequest) { return getResearchPlaylistHandler(request); }As per coding guidelines:
app/api/**/route.ts: All API routes should have JSDoc comments.Also applies to: 12-15
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@app/api/research/playlist/route.ts` around lines 5 - 7, Replace the empty JSDoc blocks above the route handler functions with concise, endpoint-specific JSDoc for the exported handlers (e.g., GET and POST functions) that describes the endpoint purpose, accepted parameters or request body shape, expected response shape/status codes, authentication/authorization requirements, and possible error conditions; update the two blank comment blocks to include summary, `@param` for Request/NextRequest where applicable, `@returns` for Response/NextResponse, and any side effects so future readers and automated linters have meaningful documentation.app/api/research/genres/route.ts (1)
5-7: Fill in the JSDoc placeholders with actual route contract details.Lines 5-7 and Lines 12-15 are empty blocks today, which makes the route self-documentation effectively missing.
📝 Proposed doc update
/** - * + * OPTIONS /api/research/genres + * + * Handles CORS preflight for the genres research endpoint. */ export async function OPTIONS() { return new NextResponse(null, { status: 200, headers: getCorsHeaders() }); } /** - * + * GET /api/research/genres + * + * Returns available genres from the research provider. + * * `@param` request */ export async function GET(request: NextRequest) { return getResearchGenresHandler(request); }As per coding guidelines:
app/api/**/route.ts: All API routes should have JSDoc comments.Also applies to: 12-15
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@app/api/research/genres/route.ts` around lines 5 - 7, Replace the empty JSDoc blocks in this route with concrete contract details: update the top-of-file JSDoc (first empty block) to include a short description of the route purpose, HTTP method(s) it supports, expected path, authorization requirements, and primary response shape; then update the second empty JSDoc (lines 12-15) immediately above the exported handler/function to document parameters (path/query/body), example request/response types or status codes, and any error conditions. Reference the route's exported handler (the default export or the named GET/POST function in this file) and ensure the JSDoc contains method, input contract, and output contract so the route is fully self-documented.app/api/research/metrics/route.ts (1)
1-18: Consider extracting a shared research route factory to reduce boilerplate.This route repeats the same
OPTIONS+ delegatedGETstructure used across many files; centralizing it will reduce copy/paste drift.♻️ Refactor sketch
-import { NextRequest, NextResponse } from "next/server"; -import { getCorsHeaders } from "@/lib/networking/getCorsHeaders"; +import { createResearchGetRoute } from "@/lib/research/createResearchGetRoute"; import { getResearchMetricsHandler } from "@/lib/research/getResearchMetricsHandler"; -/** - * - */ -export async function OPTIONS() { - return new NextResponse(null, { status: 200, headers: getCorsHeaders() }); -} - -/** - * - * `@param` request - */ -export async function GET(request: NextRequest) { - return getResearchMetricsHandler(request); -} +export const { OPTIONS, GET } = createResearchGetRoute(getResearchMetricsHandler);// lib/research/createResearchGetRoute.ts import { NextRequest, NextResponse } from "next/server"; import { getCorsHeaders } from "@/lib/networking/getCorsHeaders"; export function createResearchGetRoute( handler: (request: NextRequest) => Promise<NextResponse>, ) { return { async OPTIONS() { return new NextResponse(null, { status: 200, headers: getCorsHeaders() }); }, async GET(request: NextRequest) { return handler(request); }, }; }As per coding guidelines "
**/*.{ts,tsx}: Extract shared logic into reusable utilities following Don't Repeat Yourself (DRY) principle".🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@app/api/research/metrics/route.ts` around lines 1 - 18, This route duplicates the common OPTIONS + delegated GET pattern; extract a reusable factory (e.g., createResearchGetRoute) that returns { async OPTIONS() { return new NextResponse(null, { status: 200, headers: getCorsHeaders() }); }, async GET(request) { return handler(request); } } and update this file to export the result of createResearchGetRoute(getResearchMetricsHandler) so OPTIONS and GET behavior is centralized; reference getCorsHeaders, createResearchGetRoute, and getResearchMetricsHandler when making the change.app/api/research/audience/route.ts (1)
5-17: Empty JSDoc comments need documentation.Same pattern as other route files. Add meaningful descriptions for the audience endpoint's purpose and parameters.
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@app/api/research/audience/route.ts` around lines 5 - 17, Add meaningful JSDoc comments above the OPTIONS and GET exports to describe the endpoint's purpose and behavior: document that OPTIONS returns CORS preflight with getCorsHeaders and that GET accepts a NextRequest and delegates to getResearchAudienceHandler to retrieve research audience data; include descriptions of parameters (request: NextRequest), return values (NextResponse), and any side effects or headers so the comments match the style used in other route files.app/api/research/albums/route.ts (1)
5-17: Empty JSDoc comments need meaningful descriptions.The JSDoc blocks are present but lack any actual documentation. As per coding guidelines, all API routes should have descriptive JSDoc comments explaining the endpoint's purpose, expected parameters, and response structure.
📝 Suggested JSDoc improvements
-/** - * - */ +/** + * Handles CORS preflight requests for the albums endpoint. + * `@returns` Empty response with CORS headers. + */ export async function OPTIONS() { return new NextResponse(null, { status: 200, headers: getCorsHeaders() }); } -/** - * - * `@param` request - */ +/** + * GET /api/research/albums + * Retrieves album data for a given artist from Chartmetric. + * Requires `artist` query parameter. + * + * `@param` request - The incoming Next.js request object + * `@returns` JSON response with album data or error + */ export async function GET(request: NextRequest) { return getResearchAlbumsHandler(request); }🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@app/api/research/albums/route.ts` around lines 5 - 17, Replace the empty JSDoc blocks above the exported OPTIONS and GET functions with descriptive comments: document OPTIONS to explain it responds to preflight CORS checks, returns a 200 with headers from getCorsHeaders(), and mention any headers set; document GET to describe that it accepts a NextRequest, calls getResearchAlbumsHandler(request) to fetch research album data, outline expected query parameters (if any), possible response shapes and status codes, and note that it uses NextRequest/NextResponse types; place these JSDoc comments immediately above the OPTIONS and GET function declarations to satisfy documentation guidelines.app/api/research/festivals/route.ts (1)
5-17: Empty JSDoc comments—same pattern as other routes.Populate the JSDoc blocks with meaningful endpoint documentation describing the festivals endpoint purpose and required parameters.
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@app/api/research/festivals/route.ts` around lines 5 - 17, Replace the empty JSDoc blocks above the OPTIONS and GET functions with descriptive endpoint documentation: for OPTIONS describe this endpoint supports CORS preflight and returns 200 with CORS headers; for GET document the festivals endpoint purpose (returns research festivals list), accepted inputs (the NextRequest/query parameters like page, limit, filters or any expected headers/auth token), response shape (summary of returned data items and status codes), and any errors; reference the GET function and getResearchFestivalsHandler to ensure the documented parameters match what that handler reads.app/api/research/instagram-posts/route.ts (1)
5-17: Empty JSDoc comments need meaningful descriptions.Same issue as other route files—the JSDoc blocks are scaffolded but empty. Consider documenting the endpoint's purpose and the
artistquery parameter requirement.📝 Suggested JSDoc improvements
-/** - * - */ +/** + * Handles CORS preflight requests for the Instagram posts endpoint. + */ export async function OPTIONS() { return new NextResponse(null, { status: 200, headers: getCorsHeaders() }); } -/** - * - * `@param` request - */ +/** + * GET /api/research/instagram-posts + * Returns recent Instagram posts for the given artist via Chartmetric's DeepSocial integration. + * Requires `artist` query parameter. + * + * `@param` request - The incoming Next.js request object + */ export async function GET(request: NextRequest) { return getResearchInstagramPostsHandler(request); }🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@app/api/research/instagram-posts/route.ts` around lines 5 - 17, The JSDoc blocks above the OPTIONS and GET handlers are empty; update them to document the endpoint purpose and parameters: add a meaningful description for the OPTIONS() function stating it returns CORS preflight responses, and for GET(request: NextRequest) describe that it handles fetching research Instagram posts, requires an "artist" query parameter (describe expected format and behavior), and that it delegates to getResearchInstagramPostsHandler; reference the functions OPTIONS, GET, and getResearchInstagramPostsHandler in the docs so readers know where the logic lives.app/api/research/insights/route.ts (1)
5-17: Empty JSDoc comments require documentation.Fill in the JSDoc blocks to describe the insights endpoint—what noteworthy insights it returns and the required
artistparameter.🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@app/api/research/insights/route.ts` around lines 5 - 17, The file contains empty JSDoc blocks above the exported handlers OPTIONS and GET; update them to document the insights endpoint by describing what notable insights the GET handler returns (e.g., trend summaries, top tracks, listener demographics, or other research insights) and the required query or route parameter "artist" that the endpoint expects; add brief descriptions for the OPTIONS function (returns CORS headers and 200) and for GET (accepts a NextRequest, validates the required artist parameter, and delegates to getResearchInsightsHandler), referencing the functions OPTIONS, GET, getResearchInsightsHandler, and getCorsHeaders so maintainers can locate and understand the behavior.lib/research/proxyToChartmetric.ts (1)
48-50: Consider wrapping JSON parsing in try/catch.
response.json()can throw if the response body isn't valid JSON, which would result in an unhandled exception rather than a structured error response.🛡️ Defensive JSON parsing
- const json = await response.json(); - - const data = json.obj !== undefined ? json.obj : json; + let json: unknown; + try { + json = await response.json(); + } catch { + return { + data: { error: "Invalid JSON response from Chartmetric" }, + status: 502, + }; + } + + const data = typeof json === "object" && json !== null && "obj" in json + ? (json as { obj: unknown }).obj + : json;🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@lib/research/proxyToChartmetric.ts` around lines 48 - 50, Wrap the call to response.json() in a try/catch inside proxyToChartmetric (around the lines creating json and data) to avoid unhandled exceptions; if JSON parsing fails, catch the error (e.g., SyntaxError), log or attach the parsing error and the raw response text, and return or throw a structured error object/response instead of letting the exception bubble (ensure downstream code that uses json/data handles this error shape). Make sure to preserve existing behavior when parsing succeeds by assigning json to data as currently done.lib/research/resolveArtist.ts (1)
12-13: JSDoc return description doesn't match actual return type.The docstring says "or null if not found" but the function returns
{ error: string }on failure, notnull. The actual discriminated union return type is correct and well-designed.📝 Fix documentation
- * `@returns` The Chartmetric artist ID, or null if not found + * `@returns` Object with `id` on success, or `error` message on failure🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@lib/research/resolveArtist.ts` around lines 12 - 13, The JSDoc for resolveArtist is inaccurate: it states the function returns "The Chartmetric artist ID, or null if not found" but the implementation returns a discriminated union that includes an error object ({ error: string }). Update the `@returns` description in the resolveArtist JSDoc to accurately describe the actual return shape (e.g., success result containing the Chartmetric artist ID or a failure object with an error string), referencing the resolveArtist function and its union return type so the docs match the implementation.lib/research/getResearchCuratorHandler.ts (2)
39-39: Validateplatformandidparameters before path interpolation.User-controlled values are directly embedded in the API path. While
idshould be alphanumeric andplatformshould match known values, there's no validation enforcing this. Consider validatingplatformagainst an allowlist and ensuringidcontains only expected characters.🛡️ Suggested validation
+const VALID_CURATOR_PLATFORMS = ["spotify", "applemusic", "deezer"] as const; + export async function getResearchCuratorHandler(request: NextRequest): Promise<NextResponse> { // ... auth validation ... const { searchParams } = new URL(request.url); const platform = searchParams.get("platform"); const id = searchParams.get("id"); if (!platform || !id) { return NextResponse.json( { status: "error", error: "platform and id parameters are required" }, { status: 400, headers: getCorsHeaders() }, ); } + if (!VALID_CURATOR_PLATFORMS.includes(platform as typeof VALID_CURATOR_PLATFORMS[number])) { + return NextResponse.json( + { status: "error", error: `Invalid platform. Must be one of: ${VALID_CURATOR_PLATFORMS.join(", ")}` }, + { status: 400, headers: getCorsHeaders() }, + ); + }🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@lib/research/getResearchCuratorHandler.ts` at line 39, In getResearchCuratorHandler, validate the incoming platform and id before interpolating them into the path passed to proxyToChartmetric: enforce platform against a small allowlist (e.g., known enum values) and ensure id matches an expected regex (alphanumeric, hyphen/underscore if allowed) and reject or return a 400 error for invalid inputs; only after validation construct the path `/curator/${platform}/${id}` and call proxyToChartmetric so no unvalidated user-controlled characters reach the backend.
14-57: Consider extracting shared handler logic for non-artist lookups.This handler duplicates the auth → credits → proxy → response pattern found in
getResearchGenresHandler,getResearchFestivalsHandler, and others. A shared utility similar tohandleArtistResearchbut for non-artist lookups could reduce boilerplate. As per coding guidelines, "Extract shared logic into reusable utilities following Don't Repeat Yourself (DRY) principle."🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@lib/research/getResearchCuratorHandler.ts` around lines 14 - 57, getResearchCuratorHandler duplicates the auth→credits→proxy→response flow used by getResearchGenresHandler, getResearchFestivalsHandler, etc.; extract that shared flow into a new utility (e.g., handleNonArtistResearch or handleResearchLookup) that accepts parameters for building the proxy path (or a path template), creditsToDeduct, and any response shaping, then update getResearchCuratorHandler to call this utility instead of inlining validateAuthContext, deductCredits, proxyToChartmetric and getCorsHeaders logic; reuse the same utility from getResearchGenresHandler/getResearchFestivalsHandler and leave handleArtistResearch untouched for artist-specific differences.lib/research/getResearchPlaylistsHandler.ts (1)
20-25: Validateplatformandstatusagainst an allowlist before path interpolation.User-controlled values are directly embedded into the Chartmetric API path without validation. This pattern (also present in
getResearchMetricsHandler.ts) could allow path injection if unexpected characters are passed. While the URL constructor provides some protection, explicit validation is safer and more defensive.🛡️ Suggested validation
+const VALID_PLATFORMS = ["spotify", "applemusic", "deezer", "amazon", "itunes"] as const; +const VALID_STATUSES = ["current", "past"] as const; + export async function getResearchPlaylistsHandler(request: NextRequest) { const { searchParams } = new URL(request.url); - const platform = searchParams.get("platform") || "spotify"; - const status = searchParams.get("status") || "current"; + const platformParam = searchParams.get("platform") || "spotify"; + const statusParam = searchParams.get("status") || "current"; + + const platform = VALID_PLATFORMS.includes(platformParam as typeof VALID_PLATFORMS[number]) + ? platformParam + : "spotify"; + const status = VALID_STATUSES.includes(statusParam as typeof VALID_STATUSES[number]) + ? statusParam + : "current";🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@lib/research/getResearchPlaylistsHandler.ts` around lines 20 - 25, Validate the user-controlled platform and status before interpolating them into the Chartmetric path: in getResearchPlaylistsHandler.ts (and similarly in getResearchMetricsHandler.ts) replace the current unvalidated platform and status usage by checking the platform and status variables against explicit allowlists (e.g., ALLOWED_PLATFORMS and ALLOWED_STATUSES), and if a value is not allowed either coerce to a safe default (e.g., "spotify"/"current") or return a 400 error; do this check before calling handleArtistResearch and use the validated values when building the cmId => `/artist/${cmId}/${platform}/${status}/playlists` path. Ensure the validation uses the exact variable names platform and status so the substitution is always from trusted values.lib/research/getResearchPlaylistHandler.ts (1)
19-39: Validateplatformandidparameters to prevent path injection.Same concern as
getResearchCuratorHandler- user-controlled values are directly interpolated into the API path on line 39. Theplatformshould be validated against known streaming platforms, andidshould be validated to contain only alphanumeric characters or hyphens.🛡️ Suggested validation
+const VALID_PLAYLIST_PLATFORMS = ["spotify", "applemusic", "deezer", "amazon"] as const; +const PLAYLIST_ID_PATTERN = /^[\w-]+$/; + // After presence check: + if (!VALID_PLAYLIST_PLATFORMS.includes(platform as typeof VALID_PLAYLIST_PLATFORMS[number])) { + return NextResponse.json( + { status: "error", error: "Invalid platform" }, + { status: 400, headers: getCorsHeaders() }, + ); + } + + if (!PLAYLIST_ID_PATTERN.test(id)) { + return NextResponse.json( + { status: "error", error: "Invalid playlist ID format" }, + { status: 400, headers: getCorsHeaders() }, + ); + }🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@lib/research/getResearchPlaylistHandler.ts` around lines 19 - 39, Validate incoming query params in getResearchPlaylistHandler before calling proxyToChartmetric: ensure platform matches a whitelist of known streaming platforms (e.g., an array like ["spotify","apple","youtube","deezer"]) and ensure id matches a strict pattern (only alphanumerics, hyphens, underscores or whatever spec you choose, e.g., /^[A-Za-z0-9_-]+$/). If either check fails return a 400 NextResponse with an error message and CORS headers (same shape used elsewhere). Only after both validations pass call proxyToChartmetric(`/playlist/${platform}/${id}`) so untrusted input cannot inject paths. Ensure the validation is done prior to deductCredits or bail out early as appropriate.lib/research/getResearchLookupHandler.ts (2)
61-69: Response spreading pattern is duplicated across handlers.This exact pattern (
typeof result.data === "object" && result.data !== null ? result.data : { data: result.data }) appears in multiple handlers. Consider extracting a shared utility likeformatProxyResponse(result)to consolidate this logic.♻️ Suggested utility extraction
Create a shared utility in
lib/research/formatProxyResponse.ts:export function formatProxyResponse(data: unknown): Record<string, unknown> { return typeof data === "object" && data !== null ? (data as Record<string, unknown>) : { data }; }Then simplify handlers:
+import { formatProxyResponse } from "@/lib/research/formatProxyResponse"; return NextResponse.json( - { - status: "success", - ...(typeof result.data === "object" && result.data !== null - ? result.data - : { data: result.data }), - }, + { status: "success", ...formatProxyResponse(result.data) }, { status: 200, headers: getCorsHeaders() }, );🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@lib/research/getResearchLookupHandler.ts` around lines 61 - 69, The response-spreading logic repeated in getResearchLookupHandler (the ternary checking typeof result.data === "object" && result.data !== null and spreading result.data or wrapping it as { data: result.data }) should be extracted to a shared utility (e.g., formatProxyResponse) and used where NextResponse.json is called; create a function like formatProxyResponse(data: unknown): Record<string, unknown> that returns data if it's a non-null object or { data } otherwise, then replace the inline ternary in the NextResponse.json call (and other handlers using the same pattern) with formatProxyResponse(result.data) to centralize the logic.
43-50: Same catch block observation as the search handler.Consider differentiating between "No credits usage found" and "Insufficient credits" errors from
deductCreditsfor better error messaging.🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@lib/research/getResearchLookupHandler.ts` around lines 43 - 50, The catch block calling deductCredits in getResearchLookupHandler should capture the thrown error (catch (err)) and distinguish between a "No credits usage found" error and an "Insufficient credits" error (by checking err.code or err.message or a custom error class from deductCredits); return a 404/appropriate status and JSON {status:"error", error:"No credits usage found"} when the no-usage condition is detected, otherwise return the 402 JSON {status:"error", error:"Insufficient credits"} for insufficient funds—update deductCredits usage handling accordingly so responses accurately reflect the error type.lib/research/getResearchSearchHandler.ts (1)
31-38: Catch block masks error details.The
catchblock assumes all errors fromdeductCreditsmean "Insufficient credits," but perlib/credits/deductCredits.ts, it may also throw "No credits usage found for this account." Logging or differentiating the error message would improve debuggability.💡 Suggested improvement
try { await deductCredits({ accountId, creditsToDeduct: 5 }); - } catch { + } catch (error) { + const message = error instanceof Error ? error.message : "Insufficient credits"; return NextResponse.json( - { status: "error", error: "Insufficient credits" }, + { status: "error", error: message }, { status: 402, headers: getCorsHeaders() }, ); }🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@lib/research/getResearchSearchHandler.ts` around lines 31 - 38, The catch block in getResearchSearchHandler around the call to deductCredits currently swallows the thrown error and always returns "Insufficient credits"; change it to catch the error object (e), log or include e.message, and branch on e.message (or error type) from deductCredits to return an appropriate response (e.g., 402 for insufficient credits, a 4xx/404 for "No credits usage found for this account") while ensuring getCorsHeaders() is preserved; update the handler's error response to include the actual error message for better debuggability.lib/research/getResearchDiscoverHandler.ts (2)
26-33: Same catch block observation applies here.The catch block masks the specific error from
deductCredits. Consider preserving error details as suggested in the other handlers.🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@lib/research/getResearchDiscoverHandler.ts` around lines 26 - 33, Change the catch to capture the thrown error from deductCredits (e.g., catch (err)) and include the error details when returning the NextResponse.json so the client and logs preserve the underlying reason; update the response payload returned by NextResponse.json in getResearchDiscoverHandler to include the actual error message (err.message or String(err)) alongside "Insufficient credits" and, if available in this module, log the full error via the existing logger before returning, keeping the CORS headers from getCorsHeaders().
64-72: Same response spreading pattern—consolidation opportunity.This is the same spreading logic seen in the other handlers. A shared
formatProxyResponseutility would reduce duplication.🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@lib/research/getResearchDiscoverHandler.ts` around lines 64 - 72, The response construction in getResearchDiscoverHandler.ts duplicates the spreading logic used elsewhere when returning NextResponse.json (specifically the conditional spread of result.data vs { data: result.data }); extract this into a shared utility (e.g., formatProxyResponse) that accepts the proxy result and returns the normalized response body, then replace the inline spreading in getResearchDiscoverHandler (and other handlers) to call formatProxyResponse and pass its result into NextResponse.json with the existing status and headers.lib/research/getResearchTrackHandler.ts (2)
16-82: Handler exceeds 50-line guideline and performs two distinct operations.This handler orchestrates search + detail fetch, making it longer than the 50-line limit and arguably violating SRP. Consider splitting into smaller functions or creating a
searchAndFetchTrackhelper.♻️ Suggested decomposition
// In a separate file or as local helpers: async function searchTrackByName(q: string): Promise<{ trackId: number } | null> { const result = await proxyToChartmetric("/search", { q, type: "tracks", limit: "1" }); if (result.status !== 200) return null; const data = result.data as { tracks?: Array<{ id: number }> }; return data?.tracks?.[0] ? { trackId: data.tracks[0].id } : null; } async function fetchTrackDetails(trackId: number): Promise<ProxyResult> { return proxyToChartmetric(`/track/${trackId}`); }This keeps the main handler focused on orchestration and error responses.
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@lib/research/getResearchTrackHandler.ts` around lines 16 - 82, The getResearchTrackHandler is too long and mixes two responsibilities (search + detail fetch); extract the search and detail-fetch logic into helpers (e.g., create searchTrackByName(q: string) that calls proxyToChartmetric("/search", { q, type: "tracks", limit: "1" }) and returns a trackId or null, and fetchTrackDetails(trackId: number) that calls proxyToChartmetric(`/track/${trackId}`) ), then simplify getResearchTrackHandler to validate auth, call deductCredits, call searchTrackByName to get trackId, handle not-found/error responses, then call fetchTrackDetails and return the final response; keep existing error responses and headers (getCorsHeaders) and preserve calls to validateAuthContext and deductCredits.
31-38: Credits deducted before API calls complete.Credits are deducted at line 32, but two sequential API calls follow. If the search succeeds but the detail fetch fails (rate limit, timeout, etc.), the user loses credits without receiving the track details. This is a known tradeoff (per context snippets), but worth documenting or considering a deferred-deduction pattern for multi-call handlers.
Also applies to: 40-51, 63-71
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@lib/research/getResearchTrackHandler.ts` around lines 31 - 38, deductCredits is called before the subsequent external API calls, so if the search succeeds but the detail fetch fails the user still loses credits; change getResearchTrackHandler to either defer calling deductCredits until after all dependent API calls (e.g., after search+detail fetch succeed) or implement a compensating refund flow by calling a refundCredits/{creditRefund} helper when the detail fetch fails, and ensure the error path that returns NextResponse.json({ status: "error", ... }, { status: 402, headers: getCorsHeaders() }) triggers that refund; locate usages of deductCredits and the error return blocks (the try/catch around deductCredits and the later failure branches) and update them to use the deferred-deduction or refund approach consistently for the other similar blocks noted (lines 40-51, 63-71).
ℹ️ Review info
⚙️ Run configuration
Configuration used: Path: .coderabbit.yaml
Review profile: CHILL
Plan: Pro
Run ID: 48c2e50f-a8f0-4ff7-b53c-96694a3a874d
⛔ Files ignored due to path filters (3)
lib/chartmetric/__tests__/getChartmetricToken.test.tsis excluded by!**/*.test.*,!**/__tests__/**and included bylib/**lib/research/__tests__/proxyToChartmetric.test.tsis excluded by!**/*.test.*,!**/__tests__/**and included bylib/**lib/research/__tests__/resolveArtist.test.tsis excluded by!**/*.test.*,!**/__tests__/**and included bylib/**
📒 Files selected for processing (45)
app/api/research/albums/route.tsapp/api/research/audience/route.tsapp/api/research/career/route.tsapp/api/research/cities/route.tsapp/api/research/curator/route.tsapp/api/research/discover/route.tsapp/api/research/festivals/route.tsapp/api/research/genres/route.tsapp/api/research/insights/route.tsapp/api/research/instagram-posts/route.tsapp/api/research/lookup/route.tsapp/api/research/metrics/route.tsapp/api/research/playlist/route.tsapp/api/research/playlists/route.tsapp/api/research/profile/route.tsapp/api/research/route.tsapp/api/research/similar/route.tsapp/api/research/track/route.tsapp/api/research/tracks/route.tsapp/api/research/urls/route.tslib/chartmetric/getChartmetricToken.tslib/content/contentTemplates.tslib/research/getResearchAlbumsHandler.tslib/research/getResearchAudienceHandler.tslib/research/getResearchCareerHandler.tslib/research/getResearchCitiesHandler.tslib/research/getResearchCuratorHandler.tslib/research/getResearchDiscoverHandler.tslib/research/getResearchFestivalsHandler.tslib/research/getResearchGenresHandler.tslib/research/getResearchInsightsHandler.tslib/research/getResearchInstagramPostsHandler.tslib/research/getResearchLookupHandler.tslib/research/getResearchMetricsHandler.tslib/research/getResearchPlaylistHandler.tslib/research/getResearchPlaylistsHandler.tslib/research/getResearchProfileHandler.tslib/research/getResearchSearchHandler.tslib/research/getResearchSimilarHandler.tslib/research/getResearchTrackHandler.tslib/research/getResearchTracksHandler.tslib/research/getResearchUrlsHandler.tslib/research/handleArtistResearch.tslib/research/proxyToChartmetric.tslib/research/resolveArtist.ts
| import { NextRequest, NextResponse } from "next/server"; | ||
| import { getCorsHeaders } from "@/lib/networking/getCorsHeaders"; | ||
| import { getResearchDiscoverHandler } from "@/lib/research/getResearchDiscoverHandler"; | ||
|
|
||
| /** | ||
| * | ||
| */ | ||
| export async function OPTIONS() { | ||
| return new NextResponse(null, { status: 200, headers: getCorsHeaders() }); | ||
| } | ||
|
|
||
| /** | ||
| * | ||
| * @param request | ||
| */ | ||
| export async function GET(request: NextRequest) { | ||
| return getResearchDiscoverHandler(request); | ||
| } |
There was a problem hiding this comment.
🧩 Analysis chain
🏁 Script executed:
#!/bin/bash
set -euo pipefail
# Locate route and handler symbols
fd --type f 'route.ts' app/api/research | sort
rg -n 'getResearchDiscoverHandler|/api/research/discover'
# Find tests referencing discover endpoint/handler
rg -n --iglob '*{test,spec}*.{ts,tsx,js,jsx}' \
'/api/research/discover|getResearchDiscoverHandler'
# Check for explicit error-path assertions (status codes/messages)
rg -n --iglob '*{test,spec}*.{ts,tsx,js,jsx}' \
'discover.*(400|401|402|500)|status\s*[:=]\s*(400|401|402|500)|insufficient|invalid|missing'Repository: recoupable/api
Length of output: 1160
🏁 Script executed:
cat -n lib/research/getResearchDiscoverHandler.tsRepository: recoupable/api
Length of output: 2924
Add comprehensive test coverage and implement Zod query validation for this endpoint.
Missing test coverage for success and error paths (credit insufficiency, auth failure, proxy failures). Additionally, query parameters should be validated using a Zod schema (validateDiscoverQuery.ts) rather than manually parsed. The route file's JSDoc comments are empty and should document the endpoint's parameters and behavior.
The handler correctly uses validateAuthContext() and includes proper error handling, but these should be covered by tests. Create:
- Tests covering auth success/failure, credit success/failure, and proxy success/failure paths
- A
lib/research/validateDiscoverQuery.tsfile exporting a Zod schema and inferred type forcountry,genre,sort,limit,sp_monthly_listeners_min,sp_monthly_listeners_maxparameters - JSDoc comments in the route file documenting the endpoint
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@app/api/research/discover/route.ts` around lines 1 - 18, Add Zod-based query
validation and comprehensive tests: create lib/research/validateDiscoverQuery.ts
exporting a Zod schema and inferred TypeScript type for the query params
(country, genre, sort, limit, sp_monthly_listeners_min,
sp_monthly_listeners_max) and update the discover handler to use that schema
instead of manual parsing (reference getResearchDiscoverHandler and the exported
schema name). Add JSDoc to the route file (the exported OPTIONS and GET
handlers) documenting accepted query parameters, auth requirements, and possible
responses. Add tests that exercise success and error paths for auth
(validateAuthContext), credit checks, and proxy failures by invoking the GET
route or directly calling getResearchDiscoverHandler with mocked dependencies,
and include tests validating the schema rejectsbad inputs and accepts valid
ones.
There was a problem hiding this comment.
Fixed — JSDoc rewritten with meaningful one-liner descriptions.
There was a problem hiding this comment.
@sidneyswift, thanks for updating the JSDoc! That addresses the documentation concern.
Two items from the original review are still outstanding:
-
Zod-based query validation — query parameters (
country,genre,sort,limit,sp_monthly_listeners_min,sp_monthly_listeners_max) are currently parsed manually ingetResearchDiscoverHandler. Per the coding guidelines and the learning fromAGENTS.md, all endpoints should use Zod for input parsing. Alib/research/validateDiscoverQuery.tsexporting a schema would make validation consistent and type-safe. -
Test coverage — the learning for this repo requires tests covering all success and error paths (auth failure, insufficient credits, proxy failure, and happy path). The discover endpoint currently has no tests.
Would you like to address these in this PR, or shall I open a follow-up issue to track them?
🧠 Learnings used
Learnt from: CR
Repo: recoupable/api PR: 0
File: AGENTS.md:0-0
Timestamp: 2026-02-17T19:09:32.948Z
Learning: Applies to app/api/**/route.ts : Write tests for new API endpoints covering all success and error paths
Learnt from: CR
Repo: recoupable/api PR: 0
File: AGENTS.md:0-0
Timestamp: 2026-02-17T19:09:32.948Z
Learning: Applies to app/api/**/route.ts : All API routes should have JSDoc comments
Learnt from: CR
Repo: recoupable/api PR: 0
File: AGENTS.md:0-0
Timestamp: 2026-02-17T19:09:32.948Z
Learning: Applies to app/api/**/route.ts : All API endpoints should use a validate function for input parsing using Zod for schema validation
There was a problem hiding this comment.
Actionable comments posted: 2
🧹 Nitpick comments (1)
lib/research/getResearchPlaylistHandler.ts (1)
14-81: This handler is doing too much in one place.Auth, param validation, credit deduction, non-numeric ID resolution, upstream proxying, and response shaping are all packed into one 68-line function. Extracting something like
resolvePlaylistId()plus a small shared error-response helper would make the branches easier to test and keep this closer to the repo's handler conventions.As per coding guidelines,
lib/**/*.ts:Apply Single Responsibility Principle (SRP): one exported function per file; each file should do one thing wellandFor domain functions, ensure: Single responsibility per function,Keep functions under 50 lines, andDRY: Consolidate similar logic into shared utilities.🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@lib/research/getResearchPlaylistHandler.ts` around lines 14 - 81, getResearchPlaylistHandler is doing too many responsibilities; split its logic by extracting a resolvePlaylistId(requestPlatform, id, proxyToChartmetric) function to encapsulate the non-numeric ID lookup and playlistId resolution, and add a small helper like makeErrorResponse(payload, status) to centralize JSON error responses (used instead of repeating NextResponse.json(..., { headers: getCorsHeaders() })). Move the existing credit check (deductCredits), auth (validateAuthContext), and final proxy call (proxyToChartmetric(`/playlist/${platform}/${playlistId}`)) to orchestrate these smaller functions so getResearchPlaylistHandler becomes a thin coordinator under 50 lines, reusing resolvePlaylistId and makeErrorResponse for testable, single-responsibility units while preserving existing symbols: validateAuthContext, deductCredits, proxyToChartmetric, getCorsHeaders.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In `@lib/research/getResearchPlaylistHandler.ts`:
- Around line 30-37: The current try/catch around deductCredits treats any
thrown error as "Insufficient credits"; update the handler so it inspects the
error thrown by deductCredits and only returns NextResponse.json({ status:
"error", error: "Insufficient credits" }, { status: 402, headers:
getCorsHeaders() }) when the error explicitly indicates a
low-balance/insufficient-credits condition, while for other errors (e.g.,
missing credits record or unexpected failures) log the error and return a 500
response (NextResponse.json with status 500 and getCorsHeaders()). Locate the
deductCredits call in getResearchPlaylistHandler and implement error branching
based on the error type/message or a custom error class returned by
deductCredits. Ensure error logging includes the original error for non-402
cases.
- Around line 41-58: The search branch that calls proxyToChartmetric("/search",
...) must check the upstream response status before assuming playlists is
missing; update the logic in getResearchPlaylistHandler to inspect
searchResult.status (or equivalent) and if it is not a successful 2xx code,
return a NextResponse.json containing the upstream error message/body and use
the same status code (and include getCorsHeaders()), instead of falling through
to the 404 “No playlist found” message; keep using the existing
proxyToChartmetric call and playlists extraction but short-circuit on non-OK
searchResult.status to propagate provider errors correctly.
---
Nitpick comments:
In `@lib/research/getResearchPlaylistHandler.ts`:
- Around line 14-81: getResearchPlaylistHandler is doing too many
responsibilities; split its logic by extracting a
resolvePlaylistId(requestPlatform, id, proxyToChartmetric) function to
encapsulate the non-numeric ID lookup and playlistId resolution, and add a small
helper like makeErrorResponse(payload, status) to centralize JSON error
responses (used instead of repeating NextResponse.json(..., { headers:
getCorsHeaders() })). Move the existing credit check (deductCredits), auth
(validateAuthContext), and final proxy call
(proxyToChartmetric(`/playlist/${platform}/${playlistId}`)) to orchestrate these
smaller functions so getResearchPlaylistHandler becomes a thin coordinator under
50 lines, reusing resolvePlaylistId and makeErrorResponse for testable,
single-responsibility units while preserving existing symbols:
validateAuthContext, deductCredits, proxyToChartmetric, getCorsHeaders.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: Path: .coderabbit.yaml
Review profile: CHILL
Plan: Pro
Run ID: 4bdd1005-5f24-465d-b876-8895669f56f2
📒 Files selected for processing (1)
lib/research/getResearchPlaylistHandler.ts
There was a problem hiding this comment.
Actionable comments posted: 8
♻️ Duplicate comments (1)
lib/research/getResearchDiscoverHandler.ts (1)
50-53:⚠️ Potential issue | 🟠 MajorBug:
sp_ml[]min value is lost when both bounds are specified.This issue was previously flagged and remains unresolved. When both
mlMinandmlMaxare provided, line 53 overwrites line 52's assignment becauseproxyToChartmetricusesurl.searchParams.set()(which replaces rather than appends). Only the max listener bound reaches Chartmetric, effectively breaking the range filter.Two options to fix:
- Modify
proxyToChartmetricto acceptRecord<string, string | string[]>and useappend()for array values- Build the URL differently in this handler if Chartmetric expects a different format (comma-separated, etc.)
🔧 Option 1: Update proxyToChartmetric signature
In
lib/research/proxyToChartmetric.ts:export async function proxyToChartmetric( path: string, - queryParams?: Record<string, string>, + queryParams?: Record<string, string | string[]>, ): Promise<ProxyResult> { // ... if (queryParams) { for (const [key, value] of Object.entries(queryParams)) { - if (value !== undefined && value !== "") { - url.searchParams.set(key, value); + if (value !== undefined && value !== "") { + if (Array.isArray(value)) { + for (const v of value) { + url.searchParams.append(key, v); + } + } else { + url.searchParams.set(key, value); + } } } }Then in this handler:
const mlMin = searchParams.get("sp_monthly_listeners_min"); const mlMax = searchParams.get("sp_monthly_listeners_max"); - if (mlMin) params["sp_ml[]"] = mlMin; - if (mlMax) params["sp_ml[]"] = mlMax; + const mlValues = [mlMin, mlMax].filter(Boolean) as string[]; + if (mlValues.length > 0) params["sp_ml[]"] = mlValues;🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@lib/research/getResearchDiscoverHandler.ts` around lines 50 - 53, The handler is overwriting the sp_ml[] param when both mlMin and mlMax are present because proxyToChartmetric uses url.searchParams.set(); update proxyToChartmetric to accept Record<string, string | string[]> and, when a value is an array, call url.searchParams.append() for each element, then in getResearchDiscoverHandler (symbols: mlMin, mlMax, params and params["sp_ml[]"]) set params["sp_ml[]"] = [mlMin, mlMax] (or a single string when only one bound exists) so both bounds are sent to Chartmetric.
🧹 Nitpick comments (1)
lib/research/postResearchDeepHandler.ts (1)
15-71: Extract shared POST research orchestration into a reusable utility.This handler repeats the same auth → parse → validate → deduct → execute → map-error pattern used in other research POST handlers. Consolidating this flow will reduce drift and fix bugs once.
As per coding guidelines: "Extract shared logic into reusable utilities following Don't Repeat Yourself (DRY) principle."
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@lib/research/postResearchDeepHandler.ts` around lines 15 - 71, Extract the repeated auth→parse→validate→deduct→execute→map-error flow from postResearchDeepHandler into a reusable utility (e.g., handlePostResearchRequest) that accepts the NextRequest, creditsToDeduct, an executor callback (the core execution like chatWithPerplexity), and any operation name for logs; move the calls to validateAuthContext, request.json parsing, query presence check, deductCredits, and unified NextResponse creation (preserving getCorsHeaders and the same status codes for 400/402/500/200) into that utility, then refactor postResearchDeepHandler to call this utility with creditsToDeduct=25 and an executor that invokes chatWithPerplexity([{ role: "user", content: body.query }], "sonar-deep-research") and maps the executor result to the { status:"success", content, citations } shape while letting the utility handle error mapping.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In `@app/api/research/web/route.ts`:
- Around line 12-19: Add endpoint tests covering the new POST /api/research/web
route by exercising the exported POST(request: NextRequest) wrapper and the
underlying postResearchWebHandler; create tests for (1) auth failure (mock
invalid/absent auth and assert 401), (2) invalid JSON/body validation (send
malformed or missing fields and assert 400 with validation errors), (3)
insufficient credits (mock user/credits check to return insufficient and assert
the appropriate 402/403 response), (4) provider failure (mock the external
provider call used in postResearchWebHandler to throw or return an error and
assert the handler returns a 5xx or mapped error response), and (5) successful
path (mock auth, credits, and provider to return expected data and assert the
success status and response shape). Ensure you mock dependencies used by
postResearchWebHandler (auth, credits check, and provider client) and verify
response status codes and JSON schema for each scenario.
In `@lib/exa/searchPeople.ts`:
- Around line 27-47: The function searchPeople currently documents a "max 100"
for numResults but doesn't enforce it; clamp numResults to a safe range (e.g.,
Math.max(1, Math.min(numResults, 100))) inside the searchPeople function before
composing the request body so the POST always sends a value between 1 and 100
(use the clamped variable in the body instead of the raw numResults).
In `@lib/research/getResearchDiscoverHandler.ts`:
- Around line 26-33: The catch block in getResearchDiscoverHandler currently
hides the real failure from deductCredits; change the catch to capture the
thrown error (e.g., catch (err)) and return the original error message in the
JSON response while keeping the 402 status and CORS headers (use
getCorsHeaders()); reference deductCredits to locate the source and ensure you
use err.message or String(err) so the actual message like "No credits usage
found for this account" is preserved instead of always returning "Insufficient
credits".
In `@lib/research/getResearchPlaylistsHandler.ts`:
- Line 35: The long boolean expression assigned to hasFilters is causing
Prettier to fail; refactor the assignment in getResearchPlaylistsHandler by
replacing the long OR chain (sp.get("editorial") || sp.get("indie") ||
sp.get("majorCurator") || sp.get("personalized") || sp.get("chart")) with a
shorter, formatted expression such as using an array and Array.prototype.some:
e.g., const hasFilters =
["editorial","indie","majorCurator","personalized","chart"].some(k =>
sp.get(k)); this keeps the logic identical (calls sp.get for each key) while
allowing the formatter to wrap lines cleanly.
- Around line 35-47: The code ignores a caller-provided popularIndie value
because popularIndie isn't included in the hasFilters check or the
explicit-filter branch; update the logic so popularIndie is treated like the
other filters: add sp.get("popularIndie") to the hasFilters expression and
inside the explicit branch set params.popularIndie = sp.get("popularIndie")!
when present (leaving the else block to still set params.popularIndie = "true"
as the default).
In `@lib/research/postResearchDeepHandler.ts`:
- Around line 39-46: The code currently calls deductCredits({ accountId,
creditsToDeduct: 25 }) before the external provider call and returns a blanket
402 on any failure; move the deductCredits call to after the provider call
succeeds (so failed research doesn't consume credits), and in the deductCredits
try/catch inspect the thrown error to distinguish an "insufficient credits"
condition from other operational/storage errors — return
NextResponse.json({status:"error", error:"Insufficient credits"}, {status:402,
headers:getCorsHeaders()}) only for the genuine insufficient-balance error and
return a 5xx NextResponse.json with getCorsHeaders() for other errors; apply the
same change for the second deduction site referenced around lines 48-52 and use
the existing deductCredits, NextResponse.json and getCorsHeaders symbols to
locate and update the code paths.
- Around line 22-37: The handler currently parses request.json() into body and
only checks body.query truthiness; replace this with Zod schema validation by
creating a schema (e.g., const PostResearchSchema = z.object({ query:
z.string().min(1).transform(s => s.trim()).refine(s => s.length > 0) })) and run
it via the shared validate function (validate(PostResearchSchema, await
request.json())) instead of the try/catch + if (!body.query) block; on
validation failure return the same NextResponse.json error with getCorsHeaders
and on success use the validated.value.query (trimmed) for downstream logic so
the input shape and non-empty trimmed query are enforced.
In `@lib/research/postResearchPeopleHandler.ts`:
- Around line 23-24: The request body is not being schema-validated so
num_results can exceed allowed bounds; add a Zod validation step in
postResearchPeopleHandler (or a shared validate function) that defines body: {
query: string().optional(), num_results:
number().int().min(1).max(50).optional().default(10) } (use the documented max
of 50), call validate/parse before using the body variable, and replace usages
of the raw body.num_results with the validated value so Exa always receives a
bounded integer.
---
Duplicate comments:
In `@lib/research/getResearchDiscoverHandler.ts`:
- Around line 50-53: The handler is overwriting the sp_ml[] param when both
mlMin and mlMax are present because proxyToChartmetric uses
url.searchParams.set(); update proxyToChartmetric to accept Record<string,
string | string[]> and, when a value is an array, call url.searchParams.append()
for each element, then in getResearchDiscoverHandler (symbols: mlMin, mlMax,
params and params["sp_ml[]"]) set params["sp_ml[]"] = [mlMin, mlMax] (or a
single string when only one bound exists) so both bounds are sent to
Chartmetric.
---
Nitpick comments:
In `@lib/research/postResearchDeepHandler.ts`:
- Around line 15-71: Extract the repeated
auth→parse→validate→deduct→execute→map-error flow from postResearchDeepHandler
into a reusable utility (e.g., handlePostResearchRequest) that accepts the
NextRequest, creditsToDeduct, an executor callback (the core execution like
chatWithPerplexity), and any operation name for logs; move the calls to
validateAuthContext, request.json parsing, query presence check, deductCredits,
and unified NextResponse creation (preserving getCorsHeaders and the same status
codes for 400/402/500/200) into that utility, then refactor
postResearchDeepHandler to call this utility with creditsToDeduct=25 and an
executor that invokes chatWithPerplexity([{ role: "user", content: body.query
}], "sonar-deep-research") and maps the executor result to the {
status:"success", content, citations } shape while letting the utility handle
error mapping.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: Path: .coderabbit.yaml
Review profile: CHILL
Plan: Pro
Run ID: f201516b-5a30-466b-acc5-77dcbd3f7562
📒 Files selected for processing (18)
app/api/research/deep/route.tsapp/api/research/people/route.tsapp/api/research/web/route.tslib/exa/searchPeople.tslib/research/getResearchAlbumsHandler.tslib/research/getResearchCareerHandler.tslib/research/getResearchCitiesHandler.tslib/research/getResearchDiscoverHandler.tslib/research/getResearchFestivalsHandler.tslib/research/getResearchGenresHandler.tslib/research/getResearchInsightsHandler.tslib/research/getResearchPlaylistsHandler.tslib/research/getResearchSimilarHandler.tslib/research/getResearchTracksHandler.tslib/research/getResearchUrlsHandler.tslib/research/postResearchDeepHandler.tslib/research/postResearchPeopleHandler.tslib/research/postResearchWebHandler.ts
🚧 Files skipped from review as they are similar to previous changes (9)
- lib/research/getResearchAlbumsHandler.ts
- lib/research/getResearchInsightsHandler.ts
- lib/research/getResearchUrlsHandler.ts
- lib/research/getResearchCareerHandler.ts
- lib/research/getResearchTracksHandler.ts
- lib/research/getResearchCitiesHandler.ts
- lib/research/getResearchGenresHandler.ts
- lib/research/getResearchSimilarHandler.ts
- lib/research/getResearchFestivalsHandler.ts
| /** | ||
| * POST /api/research/web | ||
| * | ||
| * Search the web for real-time information. | ||
| */ | ||
| export async function POST(request: NextRequest) { | ||
| return postResearchWebHandler(request); | ||
| } |
There was a problem hiding this comment.
Add endpoint tests for success and error paths.
This new route is added without corresponding endpoint tests in the provided changes. Please add coverage for at least auth failure, invalid JSON/body validation, insufficient credits, provider failure, and success response shape.
Based on learnings: "Applies to app/api/**/route.ts : Write tests for new API endpoints covering all success and error paths".
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@app/api/research/web/route.ts` around lines 12 - 19, Add endpoint tests
covering the new POST /api/research/web route by exercising the exported
POST(request: NextRequest) wrapper and the underlying postResearchWebHandler;
create tests for (1) auth failure (mock invalid/absent auth and assert 401), (2)
invalid JSON/body validation (send malformed or missing fields and assert 400
with validation errors), (3) insufficient credits (mock user/credits check to
return insufficient and assert the appropriate 402/403 response), (4) provider
failure (mock the external provider call used in postResearchWebHandler to throw
or return an error and assert the handler returns a 5xx or mapped error
response), and (5) successful path (mock auth, credits, and provider to return
expected data and assert the success status and response shape). Ensure you mock
dependencies used by postResearchWebHandler (auth, credits check, and provider
client) and verify response status codes and JSON schema for each scenario.
There was a problem hiding this comment.
Fixed — added postResearchWebHandler.test.ts with auth, validation, and happy path tests.
There was a problem hiding this comment.
Rate Limit Exceeded
@sidneyswift have exceeded the limit for the number of chat messages per hour. Please wait 2 minutes and 13 seconds before sending another message.
There was a problem hiding this comment.
Actionable comments posted: 10
Note
Due to the large number of review comments, Critical, Major severity comments were prioritized as inline comments.
🟡 Minor comments (14)
app/api/research/venues/route.ts-5-15 (1)
5-15:⚠️ Potential issue | 🟡 MinorEmpty JSDoc comments need meaningful documentation.
The JSDoc blocks are present but contain no actual documentation. Per coding guidelines, all API routes should have JSDoc comments that describe the endpoint's purpose, parameters, and response.
📝 Proposed documentation
/** - * + * Handles CORS preflight requests for the venues endpoint. + * `@returns` Empty response with CORS headers */ export async function OPTIONS() { return new NextResponse(null, { status: 200, headers: getCorsHeaders() }); } /** - * - * `@param` request + * Returns venues the artist has performed at, including capacity and location. + * Requires `artist` query parameter (name or UUID). + * `@param` request - The incoming Next.js request + * `@returns` Venue data for the specified artist */ export async function GET(request: NextRequest) {As per coding guidelines: "All API routes should have JSDoc comments".
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@app/api/research/venues/route.ts` around lines 5 - 15, The empty JSDoc blocks above the OPTIONS export (function OPTIONS) and the following handler parameter should be replaced with meaningful documentation: add a brief description of the endpoint's purpose, document the request parameter (type and expected shape or headers), and describe the response (status codes and headers, e.g., CORS preflight 200 with getCorsHeaders()). Update the JSDoc for function OPTIONS and the subsequent handler's JSDoc to follow the project's JSDoc style and include `@param` and `@returns` tags so the route's behavior is clear.lib/research/getResearchRankHandler.ts-16-16 (1)
16-16:⚠️ Potential issue | 🟡 MinorUse nullish coalescing and a typed payload for
rank.Line 16 uses
|| null, which can coerce valid falsy values, andas anydrops type safety.Proposed fix
- (data) => ({ rank: (data as any)?.artist_rank || null }), + (data) => { + const rank = (data as { artist_rank?: number | null })?.artist_rank ?? null; + return { rank }; + },As per coding guidelines
lib/**/*.ts: "Use TypeScript for type safety".🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@lib/research/getResearchRankHandler.ts` at line 16, Replace the unsafe cast and falsy-coercing fallback in the mapping that returns { rank: ... } by introducing a typed payload (e.g., an interface like ResearchRankPayload with artist_rank?: number | null) and using a safe nullish coalescing fallback; specifically, change the occurrence of (data as any)?.artist_rank || null to a typed access (data as ResearchRankPayload)?.artist_rank ?? null inside the mapping used by getResearchRankHandler so valid falsy values (0, '') are preserved and you regain compile-time type safety.lib/mcp/tools/research/registerResearchPeopleTool.ts-9-13 (1)
9-13:⚠️ Potential issue | 🟡 MinorAdd integer bounds to the
num_resultsschema.The Zod schema accepts any number, but
searchPeopledocuments a maximum of 100 results. Without bounds validation, invalid or oversized requests could reach the upstream Exa API. Add.int(),.min(1), and.max(100)constraints to enforce valid input at the MCP tool boundary.Proposed fix
num_results: z .number() + .int() + .min(1) + .max(100) .optional() .default(10) .describe("Number of results to return (default: 10)"),🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@lib/mcp/tools/research/registerResearchPeopleTool.ts` around lines 9 - 13, Update the Zod schema for num_results to enforce integer bounds so oversized or invalid requests are blocked at the MCP boundary: change the num_results definition that currently uses z.number() to include .int().min(1).max(100) (keeping .optional().default(10) and .describe("Number of results to return (default: 10)") intact) so the schema validates integer values between 1 and 100 for num_results before calling searchPeople.app/api/research/radio/route.ts-5-10 (1)
5-10:⚠️ Potential issue | 🟡 MinorEmpty JSDoc comments should be populated.
Per coding guidelines, all API routes should have JSDoc comments. These are empty and don't provide documentation value.
📝 Suggested documentation
/** - * + * CORS preflight handler for /api/research/radio */ export async function OPTIONS() { return new NextResponse(null, { status: 200, headers: getCorsHeaders() }); }🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@app/api/research/radio/route.ts` around lines 5 - 10, Populate the empty JSDoc for the OPTIONS handler by adding a concise description of its purpose and behavior, parameters (none), and return type; update the comment above export async function OPTIONS() to explain that it responds to CORS preflight requests, returns a 200 NextResponse with CORS headers from getCorsHeaders(), and note any side effects or relevant headers used so callers and maintainers understand the route.app/api/research/radio/route.ts-12-18 (1)
12-18:⚠️ Potential issue | 🟡 MinorPopulate JSDoc with endpoint description.
📝 Suggested documentation
/** - * - * `@param` request + * GET /api/research/radio + * Returns the list of radio stations tracked by Chartmetric. + * Requires authentication and deducts 5 credits. + * + * `@param` request - The incoming Next.js request + * `@returns` JSON response with { status: "success", stations: Array } or error */ export async function GET(request: NextRequest) { return getResearchRadioHandler(request); }🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@app/api/research/radio/route.ts` around lines 12 - 18, Add a descriptive JSDoc block above the exported async function GET that explains this endpoint's purpose (e.g., "Handles GET requests for the research radio endpoint"), describes the request parameter (NextRequest) and any expected query params or headers, and documents the return value (the Response returned by getResearchRadioHandler). Update the JSDoc to reference the handler function name getResearchRadioHandler and include tags like `@param` and `@returns` so readers and editors can quickly understand inputs/outputs.lib/mcp/tools/research/registerResearchCitiesTool.ts-1-1 (1)
1-1:⚠️ Potential issue | 🟡 MinorAddress Prettier formatting failure.
The pipeline indicates a Prettier formatting check failure. Run
prettier --writeon this file to resolve.🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@lib/mcp/tools/research/registerResearchCitiesTool.ts` at line 1, Prettier formatting failed for this file; run the formatter (e.g., prettier --write) on the file containing the import of McpServer (the import statement "import { McpServer } from \"@modelcontextprotocol/sdk/server/mcp.js\"") or apply Prettier rules to fix whitespace/quoting/newline issues so the file conforms to project Prettier settings; after formatting, re-run the pipeline to confirm the Prettier check passes.lib/mcp/tools/research/registerResearchSimilarTool.ts-75-79 (1)
75-79:⚠️ Potential issue | 🟡 MinorResponse normalization differs from the HTTP handler.
The HTTP handler (
getResearchSimilarHandler.tslines 37-42) returnstotalunconditionally from the data object, while this MCP tool conditionally omitstotalwhendatais an array. This inconsistency could cause confusion for consumers expecting identical shapes from both interfaces.Consider aligning the normalization logic:
♻️ Suggested alignment with HTTP handler
const data = result.data as Record<string, unknown>; return getToolResultSuccess({ artists: Array.isArray(data) ? data : data?.data || [], - total: Array.isArray(data) ? undefined : data?.total, + total: (data as Record<string, unknown>)?.total, });🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@lib/mcp/tools/research/registerResearchSimilarTool.ts` around lines 75 - 79, The tool's response normalization in registerResearchSimilarTool (around the getToolResultSuccess call) diverges from the HTTP handler getResearchSimilarHandler by conditionally omitting total when data is an array; change the normalization to match the HTTP handler by always returning total from the data object (use data?.total) and keep artists computed as Array.isArray(data) ? data : data?.data || [], so getToolResultSuccess returns the same shape as the HTTP handler.lib/mcp/tools/research/registerResearchChartsTool.ts-44-62 (1)
44-62:⚠️ Potential issue | 🟡 MinorMCP tools lack credit deduction—a pattern across all tools, not just charts.
The HTTP handler deducts 5 credits via
handleResearchRequest, but this MCP tool (and all others inlib/mcp/tools/) skip credit deduction entirely. This architectural inconsistency between HTTP and MCP interfaces creates a potential credit-bypass path. Consider whether MCP tools should share the same credit deduction logic as their HTTP counterparts for billing consistency.🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@lib/mcp/tools/research/registerResearchChartsTool.ts` around lines 44 - 62, The MCP tool handler in registerResearchChartsTool.ts currently calls proxyToChartmetric and returns getToolResultSuccess/getToolResultError without deducting credits; add a call to the shared credit-deduction routine (reuse handleResearchRequest or its underlying deductCredits function) at the start of the async (args) handler (or wrap the proxy call in handleResearchRequest) so the same 5-credit deduction occurs for MCP requests; locate the anonymous async handler, proxyToChartmetric, and the getToolResultSuccess/getToolResultError calls and ensure the credit deduction runs before the external call and that failures still return getToolResultError while not double-deducting.lib/mcp/tools/research/registerResearchUrlsTool.ts-34-35 (1)
34-35:⚠️ Potential issue | 🟡 MinorMissing status check on
proxyToChartmetricresult.Per the
proxyToChartmetriccontract (seelib/research/proxyToChartmetric.ts:5-52), non-200 responses return{ data: { error: string }, status: number }without throwing. The current implementation returnsresult.dataunconditionally, which could surface Chartmetric error payloads as successful tool results.🛡️ Suggested fix to check status
const result = await proxyToChartmetric(`/artist/${resolved.id}/urls`); + if (result.status !== 200) { + return getToolResultError( + `Request failed with status ${result.status}`, + ); + } return getToolResultSuccess(result.data);🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@lib/mcp/tools/research/registerResearchUrlsTool.ts` around lines 34 - 35, The call to proxyToChartmetric in registerResearchUrlsTool returns a non-throwing result object with status and data; update the code in the function that calls proxyToChartmetric(`/artist/${resolved.id}/urls`) to check result.status === 200 before returning getToolResultSuccess(result.data), and when status is not 200 return a failure via getToolResultError (or similar error-return helper) including the Chartmetric error payload (e.g., result.data.error) and the status so callers don't treat error responses as successes.lib/mcp/tools/research/registerResearchAudienceTool.ts-41-44 (1)
41-44:⚠️ Potential issue | 🟡 MinorMissing status check on
proxyToChartmetricresult.Same issue as in
registerResearchUrlsTool.ts— the result status isn't checked before returning data, which could propagate Chartmetric error payloads as successful tool responses.🛡️ Suggested fix
const result = await proxyToChartmetric( `/artist/${resolved.id}/${platform}-audience-stats`, ); + if (result.status !== 200) { + return getToolResultError( + `Request failed with status ${result.status}`, + ); + } return getToolResultSuccess(result.data);🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@lib/mcp/tools/research/registerResearchAudienceTool.ts` around lines 41 - 44, The call to proxyToChartmetric in registerResearchAudienceTool.ts returns a response object whose HTTP status isn't checked, so Chartmetric errors can be returned as successes; update the code after calling proxyToChartmetric(`/artist/${resolved.id}/${platform}-audience-stats`) to inspect result.status (or equivalent), and if it indicates failure return a failure tool response (e.g., via getToolResultFailure with the status and error payload) otherwise return getToolResultSuccess(result.data); mirror the same status-check pattern used in registerResearchUrlsTool.ts and reference proxyToChartmetric and getToolResultSuccess/getToolResultFailure to locate where to change.lib/mcp/tools/research/registerResearchCareerTool.ts-34-37 (1)
34-37:⚠️ Potential issue | 🟡 MinorMissing status check on
proxyToChartmetricresult.Consistent with the pattern in other MCP tools in this PR, the status code isn't validated before returning data. Add a status check for robustness.
🛡️ Suggested fix
const result = await proxyToChartmetric( `/artist/${resolved.id}/career`, ); + if (result.status !== 200) { + return getToolResultError( + `Request failed with status ${result.status}`, + ); + } return getToolResultSuccess(result.data);🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@lib/mcp/tools/research/registerResearchCareerTool.ts` around lines 34 - 37, The call to proxyToChartmetric in registerResearchCareerTool returns a response whose HTTP status is not being validated before using result.data; update the handler to check the response status (e.g., ensure result.status === 200 or a success-range) and only call getToolResultSuccess(result.data) on success, otherwise return an error path (e.g., getToolResultFailure or a similar failure helper) including the status and error payload; locate the proxyToChartmetric call and the return of getToolResultSuccess in registerResearchCareerTool to add this status check and appropriate failure return.lib/research/postResearchEnrichHandler.ts-23-47 (1)
23-47:⚠️ Potential issue | 🟡 MinorAdd validation for
processorparameter.The
body.processortype annotation suggests"base" | "core" | "ultra", but there's no runtime validation. Invalid values silently default to 5 credits (line 47). The MCP tool counterpart usesz.enum(["base", "core", "ultra"])for strict validation—this handler should be consistent.🛡️ Suggested validation
+ const validProcessors = ["base", "core", "ultra"] as const; + const processor = body.processor && validProcessors.includes(body.processor) + ? body.processor + : "base"; + - const creditCost = body.processor === "ultra" ? 25 : body.processor === "core" ? 10 : 5; + const creditCost = processor === "ultra" ? 25 : processor === "core" ? 10 : 5; try { await deductCredits({ accountId, creditsToDeduct: creditCost });🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@lib/research/postResearchEnrichHandler.ts` around lines 23 - 47, Add runtime validation for body.processor to ensure it is one of "base" | "core" | "ultra" before computing creditCost: check the parsed request body (body.processor) and if it's missing or not one of the allowed strings return a 400 JSON response (similar style to the input/schema checks). Then compute creditCost using the validated value (the existing creditCost expression referencing body.processor) so invalid values don't silently fall back to 5. Use the same error/response pattern and headers as the other early-return validations in postResearchEnrichHandler.lib/parallel/enrichEntity.ts-1-1 (1)
1-1:⚠️ Potential issue | 🟡 MinorRun Prettier to fix formatting.
Pipeline indicates Prettier check failed. Run
prettier --writeon this file to resolve.🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@lib/parallel/enrichEntity.ts` at line 1, Reformat this file with the project's Prettier settings (run `prettier --write`) to fix the formatting error reported by CI; specifically ensure the top-level constant PARALLEL_BASE_URL and surrounding code are reformatted to match the repo style so Prettier checks pass.lib/mcp/tools/research/registerResearchPlaylistsTool.ts-1-1 (1)
1-1:⚠️ Potential issue | 🟡 MinorRun Prettier to fix formatting.
Pipeline indicates Prettier check failed. Run
prettier --writeon this file to resolve.🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@lib/mcp/tools/research/registerResearchPlaylistsTool.ts` at line 1, Run Prettier to fix formatting for this file: reformat the file (e.g., run `prettier --write`) so the import line "import { McpServer } from "@modelcontextprotocol/sdk/server/mcp.js";" and the rest of registerResearchPlaylistsTool.ts conform to the project's Prettier rules; commit the reformatted file to resolve the pipeline Prettier check failure.
🧹 Nitpick comments (21)
app/api/research/rank/route.ts (1)
5-7: Replace empty JSDoc blocks with meaningful route documentation.Line 5 and Line 12 docblocks are empty; add concise endpoint/method/param descriptions.
As per coding guidelines
app/api/**/route.ts: "All API routes should have JSDoc comments".Also applies to: 12-15
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@app/api/research/rank/route.ts` around lines 5 - 7, The empty JSDoc blocks above the route handler need to be replaced with concise route documentation: add a short JSDoc before the exported route handler(s) (e.g., the GET/POST/handler function name used in this file) describing the endpoint path, HTTP method, expected query/body parameters, and the response shape/status codes; also update the second empty block (lines ~12-15) with param descriptions and an example response or error conditions so the file complies with the API route documentation guideline.lib/mcp/tools/research/registerResearchInstagramPostsTool.ts (1)
34-37: Consider normalizing response shape for consistency.Other collection-returning tools (
research_tracks,research_albums,research_insights) normalize responses into{ [collection]: Array }. This tool returnsresult.datadirectly, which may yield inconsistent response shapes across the research tool family.If the Instagram data structure permits, consider normalizing to
{ posts: Array.isArray(data) ? data : [] }for a more predictable API.♻️ Optional normalization
const result = await proxyToChartmetric( `/SNS/deepSocial/cm_artist/${resolved.id}/instagram`, ); - return getToolResultSuccess(result.data); + const data = result.data; + return getToolResultSuccess({ + posts: Array.isArray(data) ? data : [], + });🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@lib/mcp/tools/research/registerResearchInstagramPostsTool.ts` around lines 34 - 37, The Instagram tool currently returns result.data directly which causes inconsistent shapes vs other research tools; inside the registerResearchInstagramPostsTool handler (the block calling proxyToChartmetric and getToolResultSuccess) normalize the response to an object like { posts: Array.isArray(result.data) ? result.data : [] } (or appropriate property name if Instagram uses a different key) and pass that normalized object into getToolResultSuccess so the tool returns a consistent collection-shaped payload.lib/research/getResearchRadioHandler.ts (1)
20-27: Consider distinguishing credit deduction errors.The current implementation catches all exceptions from
deductCreditsand returns 402 with "Insufficient credits". IfdeductCreditscan throw for other reasons (e.g., database errors), this may mask the actual failure.♻️ More precise error handling
try { await deductCredits({ accountId, creditsToDeduct: 5 }); - } catch { + } catch (error) { + const isInsufficientCredits = error instanceof Error && + error.message.includes("Insufficient"); return NextResponse.json( - { status: "error", error: "Insufficient credits" }, - { status: 402, headers: getCorsHeaders() }, + { status: "error", error: isInsufficientCredits ? "Insufficient credits" : "Credit deduction failed" }, + { status: isInsufficientCredits ? 402 : 500, headers: getCorsHeaders() }, ); }🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@lib/research/getResearchRadioHandler.ts` around lines 20 - 27, Catch the error thrown by deductCredits({ accountId, creditsToDeduct: 5 }) into a variable and distinguish insufficient-credit failures from other failures: check for a specific error marker (e.g., error instanceof InsufficientCreditsError, error.name === 'InsufficientCreditsError', or error.code === 'INSUFFICIENT_CREDITS') and return the 402 NextResponse.json({ status: "error", error: "Insufficient credits" }, ...) only in that case; for other errors, log or capture the error and return a 500 NextResponse.json({ status: "error", error: "Internal server error" }, ...) (or rethrow) so database/unknown errors are not masked. Ensure references to deductCredits and the surrounding try/catch in getResearchRadioHandler.ts are updated accordingly.lib/mcp/tools/research/index.ts (1)
28-29: Minor: Missing blank line before JSDoc.There's no blank line between the last import (line 28) and the JSDoc comment (line 29). This is a minor formatting inconsistency but may be caught by the linter.
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@lib/mcp/tools/research/index.ts` around lines 28 - 29, Minor formatting: add a single blank line between the last import statement (import { registerResearchRadioTool } from "./registerResearchRadioTool";) and the following JSDoc block so the JSDoc is separated from imports; update the top of lib/mcp/tools/research/index.ts by inserting one empty line between that import and the /** JSDoc comment to satisfy linter/style rules.app/api/research/charts/route.ts (1)
5-10: JSDoc comments are incomplete.The JSDoc blocks are present but lack descriptions. Per coding guidelines, all API routes should have JSDoc comments. Consider adding meaningful descriptions for both handlers.
📝 Suggested documentation
/** - * + * OPTIONS handler for CORS preflight requests. */ export async function OPTIONS() { return new NextResponse(null, { status: 200, headers: getCorsHeaders() }); } /** - * - * `@param` request + * GET /api/research/charts + * + * Returns global chart positions for a given platform (Spotify, Apple Music, etc.). + * + * `@param` request - The incoming Next.js request object */ export async function GET(request: NextRequest) { return getResearchChartsHandler(request); }Also applies to: 12-18
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@app/api/research/charts/route.ts` around lines 5 - 10, The JSDoc blocks for the API route handlers (e.g., the exported async function OPTIONS and the other handlers around that block) are empty—add concise JSDoc comments above each exported handler describing what the route/handler does, its expected inputs or purpose, and the response (e.g., "Handles CORS preflight requests and returns 200 with CORS headers" for OPTIONS); apply the same pattern to the other handlers in this file so each exported function has a meaningful description, parameters/returns notes if applicable, and any relevant behavior details.lib/research/getResearchChartsHandler.ts (2)
20-51: Consider using Zod schema for input validation.Per coding guidelines, all API endpoints should use a validate function with Zod for input parsing. Currently, parameters are manually extracted without schema validation. This would improve type safety and provide better error messages.
♻️ Suggested Zod schema approach
import { z } from "zod"; const chartsQuerySchema = z.object({ platform: z.string().min(1, "platform parameter is required"), country: z.string().optional(), interval: z.string().optional(), type: z.string().optional(), latest: z.string().default("true"), }); // Then in handler: const parseResult = chartsQuerySchema.safeParse(Object.fromEntries(searchParams)); if (!parseResult.success) { return NextResponse.json( { status: "error", error: parseResult.error.issues[0].message }, { status: 400, headers: getCorsHeaders() }, ); } const { platform, country, interval, type, latest } = parseResult.data;🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@lib/research/getResearchChartsHandler.ts` around lines 20 - 51, Replace manual query extraction with Zod-based validation: define a chartsQuerySchema (using z.object with platform: z.string().min(1), country/interval/type optional, latest default "true") and run safeParse/validate on Object.fromEntries(searchParams) at the top of getResearchChartsHandler; if validation fails return a 400 NextResponse.json with the validation message and getCorsHeaders(), and only after successful parse call deductCredits({ accountId, creditsToDeduct: 5 }); then use parsed fields (platform, country, interval, type, latest) to populate params instead of reading searchParams directly. Ensure you reference chartsQuerySchema, searchParams, NextResponse, getCorsHeaders, and deductCredits when making the change.
62-65: Response structure uses nesteddatawrapper.The coding guidelines specify to keep API response bodies flat with fields at the root level, not nested inside a
datawrapper. However, this pattern appears consistent with other research handlers in this PR. If this is intentional for this endpoint family, consider documenting the exception.🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@lib/research/getResearchChartsHandler.ts` around lines 62 - 65, The response body currently nests the payload under a data property (in the return inside getResearchChartsHandler), violating the flat-root response convention; change the NextResponse.json call to place fields from result.data at the root (e.g., return { status: "success", ...result.data }) instead of { status: "success", data: result.data }, and if the nested pattern is intentional for the research handlers, update or add a short note in docs or a comment to justify the exception and make the pattern consistent across related handlers.lib/mcp/tools/research/registerResearchAudienceTool.ts (1)
40-40: Redundant fallback — Zod default already provides"instagram".The schema at line 13 uses
.default("instagram"), soargs.platformwill never beundefinedafter Zod parsing. The?? "instagram"fallback is defensive but unnecessary.✂️ Simplification
- const platform = args.platform ?? "instagram"; + const platform = args.platform;🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@lib/mcp/tools/research/registerResearchAudienceTool.ts` at line 40, The fallback is redundant because the Zod schema already sets a default; replace the defensive expression by using args.platform directly (remove the "?? 'instagram'" from the const assignment) in the registerResearchAudienceTool code so the variable simply reads the parsed value from args.platform; ensure no other code assumes undefined for platform.lib/mcp/tools/research/registerResearchEnrichTool.ts (1)
43-43: Redundant fallback — Zod default already provides"base".The schema at line 15 uses
.default("base"), soargs.processorwill always be defined after parsing. The?? "base"fallback is unnecessary.✂️ Simplification
- args.processor ?? "base", + args.processor,🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@lib/mcp/tools/research/registerResearchEnrichTool.ts` at line 43, Remove the redundant fallback by deleting the `?? "base"` after `args.processor` in the call site (the `args.processor ?? "base"` expression) since the Zod schema already sets a default of "base"; update the code in the registerResearchEnrichTool usage to pass `args.processor` directly (referencing the `args.processor` symbol and the surrounding function `registerResearchEnrichTool`) so the value comes from the parsed args without an extra fallback.app/api/research/milestones/route.ts (1)
5-15: Empty JSDoc comments provide no documentation value.The JSDoc blocks for both
OPTIONSandGEThandlers are empty placeholders. As per coding guidelines, all API routes should have meaningful JSDoc comments describing the endpoint's purpose, parameters, and response.📝 Suggested JSDoc improvements
/** - * + * OPTIONS handler for CORS preflight requests. */ export async function OPTIONS() { return new NextResponse(null, { status: 200, headers: getCorsHeaders() }); } /** - * - * `@param` request + * GET /api/research/milestones + * + * Returns an artist's activity feed — playlist adds, chart entries, + * and other notable events tracked by Chartmetric. + * + * `@param` request - The incoming request with artist query parameter */ export async function GET(request: NextRequest) {🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@app/api/research/milestones/route.ts` around lines 5 - 15, The file contains empty JSDoc blocks for the API route handlers; update the JSDoc for the exported handlers (OPTIONS and GET in route.ts) to provide a concise description of the endpoint's purpose, the incoming parameters (e.g., Request or query params), and the expected response shape/status codes; reference the OPTIONS and GET functions and include notes about CORS headers (getCorsHeaders) in the docs so consumers and maintainers understand behavior and responses.lib/research/getResearchMilestonesHandler.ts (1)
12-18: Consider defining a type for the Chartmetric response.The
(data as any)?.insightscast works but loses type safety. A minimal interface would improve maintainability and IDE support without significant effort.🔧 Optional type improvement
+interface MilestonesResponse { + insights?: unknown[]; +} + export async function getResearchMilestonesHandler(request: NextRequest) { return handleArtistResearch( request, (cmId) => `/artist/${cmId}/milestones`, undefined, - (data) => ({ milestones: (data as any)?.insights || [] }), + (data) => ({ milestones: (data as MilestonesResponse)?.insights || [] }), ); }🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@lib/research/getResearchMilestonesHandler.ts` around lines 12 - 18, Replace the untyped cast in getResearchMilestonesHandler by introducing a minimal interface for the Chartmetric response (e.g. ChartmetricMilestonesResponse with an insights: Milestone[] field) and use that type instead of any; update the mapping callback passed to handleArtistResearch to expect ChartmetricMilestonesResponse and return { milestones: response.insights || [] } (or make handleArtistResearch generic so you can call handleArtistResearch<ChartmetricMilestonesResponse>(...)). This preserves type safety while keeping the transformation logic in getResearchMilestonesHandler and requires adding the new interface (and optional Milestone type) and, if necessary, a generic parameter on handleArtistResearch.lib/mcp/tools/research/registerResearchMetricsTool.ts (1)
10-14: Consider usingz.enum()for platform validation consistency.The sibling tool
registerResearchAudienceToolusesz.enum(["instagram", "tiktok", "youtube"])to constrain platform values at the schema level. Here,sourceaccepts any string and is interpolated directly into the URL path at line 42. While Chartmetric will reject invalid platforms, schema-level validation provides better error messages and consistency across tools.♻️ Suggested improvement
const schema = z.object({ artist: z.string().describe("Artist name to research"), source: z - .string() + .enum([ + "spotify", + "instagram", + "tiktok", + "youtube_channel", + "soundcloud", + "deezer", + "twitter", + "facebook", + ]) .describe( "Platform: spotify, instagram, tiktok, youtube_channel, soundcloud, deezer, twitter, facebook, etc.", ), });🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@lib/mcp/tools/research/registerResearchMetricsTool.ts` around lines 10 - 14, The schema for `source` in `registerResearchMetricsTool` currently uses `z.string()` and should be tightened to a `z.enum()` that lists the allowed platforms (the same set used by `registerResearchAudienceTool`, e.g., "instagram", "tiktok", "youtube", etc.) so invalid platform values are rejected at schema validation instead of only by the downstream Chartmetric call; update the `source` schema to `z.enum([...])`, keep the descriptive text, and ensure any code that reads `source` (the URL path interpolation) continues to work with the enum values.lib/research/postResearchExtractHandler.ts (1)
46-53: Consider preserving the original error message fromdeductCredits.The
deductCreditsfunction throws with specific messages (e.g., "No credits usage found for this account" vs "Insufficient credits. Required: X, Available: Y"). Catching all exceptions as "Insufficient credits" loses diagnostic information.♻️ Preserve error detail
try { await deductCredits({ accountId, creditsToDeduct: 5 * body.urls.length }); } catch { + } catch (error) { return NextResponse.json( - { status: "error", error: "Insufficient credits" }, + { status: "error", error: error instanceof Error ? error.message : "Insufficient credits" }, { status: 402, headers: getCorsHeaders() }, ); }🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@lib/research/postResearchExtractHandler.ts` around lines 46 - 53, The catch for deductCredits currently swallows the original error; change the try/catch to catch the error (e.g., catch (err)) and return the NextResponse.json error payload using the original message (e.g., error: err?.message || String(err) || "Insufficient credits") while keeping the 402 status and getCorsHeaders; update references in this block (deductCredits, accountId, creditsToDeduct, NextResponse.json, getCorsHeaders) so diagnostic details from deductCredits are preserved safely.lib/mcp/tools/research/registerResearchRankTool.ts (1)
38-38: Consider using nullish coalescing for rank extraction.Using
|| nullwill convert a rank of0tonull. While Chartmetric ranks are typically 1-indexed, using?? nullis more defensive and semantically correct for "missing value" scenarios.♻️ Minor fix
- const rank = (result.data as any)?.artist_rank || null; + const rank = (result.data as any)?.artist_rank ?? null;🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@lib/mcp/tools/research/registerResearchRankTool.ts` at line 38, The current extraction uses const rank = (result.data as any)?.artist_rank || null which will coerce a valid 0 rank to null; change the fallback to the nullish coalescing operator so use (result.data as any)?.artist_rank ?? null instead — update the declaration of rank in registerResearchRankTool (the const rank binding that reads result.data.artist_rank) to use ?? null so only null/undefined become null while preserving numeric 0.lib/parallel/enrichEntity.ts (3)
80-86: Consider defensive type check foroutput.basis.The
output?.basis || []handles undefined, but ifbasisexists but isn't an array (API contract change),flatMapwill throw. A defensive check improves resilience:🛡️ Defensive type guard
- const citations = (output?.basis || []).flatMap( + const basisArray = Array.isArray(output?.basis) ? output.basis : []; + const citations = basisArray.flatMap(🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@lib/parallel/enrichEntity.ts` around lines 80 - 86, The code assumes output.basis is an array when building citations from resultData.output; add a defensive type guard before using flatMap: verify output exists and Array.isArray(output.basis) (or coerce to an empty array) to avoid runtime errors if basis is present but not an array. Update the citations computation (referencing resultData, output, and the citations variable) to use the guarded/normalized basis value so flatMap is only called on a true array.
55-60: Add type safety for the task creation response.The response from
createResponse.json()is untyped. Consider adding a minimal type assertion or validation to ensurerun_idexists with the expected type.♻️ Type-safe response handling
- const taskRun = await createResponse.json(); - const runId = taskRun.run_id; + const taskRun = (await createResponse.json()) as { run_id?: string }; + const runId = taskRun.run_id;Or for stricter validation:
interface TaskRunResponse { run_id: string; } const taskRun: TaskRunResponse = await createResponse.json();🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@lib/parallel/enrichEntity.ts` around lines 55 - 60, The code reads an untyped JSON from createResponse.json() and then accesses run_id (taskRun and runId); add type safety by defining a minimal interface (e.g., TaskRunResponse { run_id: string }) and assert/validate the parsed JSON against it, or perform a runtime check that taskRun has a string run_id before assigning runId, throwing a clear error if validation fails; apply this change around the createResponse.json() call and the taskRun/runId variables to ensure type-safe handling.
20-94: Function exceeds 50-line guideline.At ~74 lines, this function exceeds the coding guideline suggesting functions stay under 50 lines. Consider extracting the task creation (lines 32-60) and result polling (lines 62-93) into separate private helpers if this module grows.
As per coding guidelines: "Keep functions under 50 lines."
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@lib/parallel/enrichEntity.ts` around lines 20 - 94, The enrichEntity function is over the 50-line guideline—extract the task creation and result fetching logic into two private helpers to reduce size: move the POST-to-/tasks/runs block (currently creating createResponse, checking createResponse.ok, parsing taskRun and runId) into a helper named e.g. createParallelTaskRun(input, outputSchema, processor, apiKey) and move the GET-to-/tasks/runs/{runId}/result logic (timeout/404/other error handling, parsing resultData, building output and citations) into a helper named e.g. fetchParallelTaskResult(runId, timeout, apiKey); then have enrichEntity call those two helpers and return the final EnrichResult, preserving existing error messages and behavior.lib/parallel/extractUrl.ts (2)
56-56: Redundantawaitin return statement.The
awaitbeforeresponse.json()is unnecessary when immediately returning. The async function already wraps the return value in a Promise.♻️ Minor cleanup
- return await response.json(); + return response.json();Note: Some style guides prefer explicit
awaitfor consistency in error stack traces. This is a minor preference.🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@lib/parallel/extractUrl.ts` at line 56, Remove the redundant await in the return statement: replace the explicit "return await response.json()" inside the async function (the function containing the response.json() call in lib/parallel/extractUrl.ts) with "return response.json()" so the async function directly returns the Promise from response.json().
22-24: Input constraints documented but not validated.The JSDoc mentions "max 10 per request" for URLs and "max 3000 chars" for objective, but these aren't validated before the API call. Consider adding validation to fail fast with clear errors:
🛡️ Add input validation
export async function extractUrl( urls: string[], objective?: string, fullContent: boolean = false, ): Promise<ExtractResponse> { + if (urls.length === 0) { + throw new Error("At least one URL is required"); + } + if (urls.length > 10) { + throw new Error("Maximum 10 URLs allowed per request"); + } + if (objective && objective.length > 3000) { + throw new Error("Objective must be 3000 characters or fewer"); + } + const apiKey = process.env.PARALLEL_API_KEY;🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@lib/parallel/extractUrl.ts` around lines 22 - 24, The JSDoc limits (max 10 URLs and max 3000 chars for objective) are not enforced; update the exported function (e.g., extractUrl or extractUrls in lib/parallel/extractUrl.ts) to validate inputs up-front: check that urls is an array with 1–10 items, that each item is a string (optionally validate via new URL(...) to ensure well-formed URLs), and that objective, if provided, is a string of length ≤3000; if any check fails, throw a clear, synchronous error (or return a rejected Promise) with a descriptive message before making the API call so callers fail fast.lib/mcp/tools/research/registerResearchPlaylistsTool.ts (2)
60-67: Filter behavior differs wheneditorialis explicitly provided.When
editorialis explicitly set (true or false), only that filter is sent. This means:
editorial: true→ only editorial playlists (loses indie, majorCurator, popularIndie)editorial: false→ sendseditorial=falsewithout other filtersCompare with the REST handler in
getResearchPlaylistsHandler.tswhich supportsindie,majorCurator,personalized, andchartfilters individually. If this is intentional simplification for MCP, consider documenting it. Otherwise, exposing additional filter options would provide parity.🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@lib/mcp/tools/research/registerResearchPlaylistsTool.ts` around lines 60 - 67, Current behavior in registerResearchPlaylistsTool (the args -> queryParams block) only sends the editorial filter when args.editorial is defined, dropping indie/majorCurator/popularIndie; update the logic so that when args.editorial is provided you still populate other filter query params from args if present (e.g., args.indie, args.majorCurator, args.popularIndie, and any supported personalized/chart flags) instead of overwriting them, mirroring the behavior in getResearchPlaylistsHandler.ts; locate the conditional around args.editorial and change it to set queryParams.editorial = String(args.editorial) while also conditionally setting queryParams.indie, queryParams.majorCurator, queryParams.popularIndie (and personalized/chart if supported) from args when those fields are present, or document the intentional simplification if parity is not desired.
24-28: Thelimitdefault makes the conditional on line 58 always true.Since
limithas.default(20),args.limitwill always be defined and truthy (20). The conditionalif (args.limit)on line 58 will always evaluate to true, solimitis always included inqueryParams.This works correctly but is misleading. Consider removing the conditional for clarity:
♻️ Clearer implementation
const queryParams: Record<string, string> = {}; - if (args.limit) queryParams.limit = String(args.limit); + queryParams.limit = String(args.limit);🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@lib/mcp/tools/research/registerResearchPlaylistsTool.ts` around lines 24 - 28, The conditional "if (args.limit)" is misleading because args.limit always has a default 20; update registerResearchPlaylistsTool to always include the limit param instead of conditionally adding it: remove the "if (args.limit)" guard and assign queryParams.limit = args.limit (or otherwise ensure queryParams includes args.limit unconditionally). This keeps the z schema default behavior and makes the intent explicit by using args.limit and queryParams directly.
ℹ️ Review info
⚙️ Run configuration
Configuration used: Path: .coderabbit.yaml
Review profile: CHILL
Plan: Pro
Run ID: 1868c788-3137-4922-9193-f96aacc7789b
📒 Files selected for processing (45)
app/api/research/charts/route.tsapp/api/research/enrich/route.tsapp/api/research/extract/route.tsapp/api/research/milestones/route.tsapp/api/research/radio/route.tsapp/api/research/rank/route.tsapp/api/research/venues/route.tslib/mcp/tools/index.tslib/mcp/tools/research/index.tslib/mcp/tools/research/registerResearchAlbumsTool.tslib/mcp/tools/research/registerResearchArtistTool.tslib/mcp/tools/research/registerResearchAudienceTool.tslib/mcp/tools/research/registerResearchCareerTool.tslib/mcp/tools/research/registerResearchChartsTool.tslib/mcp/tools/research/registerResearchCitiesTool.tslib/mcp/tools/research/registerResearchCuratorTool.tslib/mcp/tools/research/registerResearchDiscoverTool.tslib/mcp/tools/research/registerResearchEnrichTool.tslib/mcp/tools/research/registerResearchExtractTool.tslib/mcp/tools/research/registerResearchFestivalsTool.tslib/mcp/tools/research/registerResearchGenresTool.tslib/mcp/tools/research/registerResearchInsightsTool.tslib/mcp/tools/research/registerResearchInstagramPostsTool.tslib/mcp/tools/research/registerResearchLookupTool.tslib/mcp/tools/research/registerResearchMetricsTool.tslib/mcp/tools/research/registerResearchMilestonesTool.tslib/mcp/tools/research/registerResearchPeopleTool.tslib/mcp/tools/research/registerResearchPlaylistTool.tslib/mcp/tools/research/registerResearchPlaylistsTool.tslib/mcp/tools/research/registerResearchRadioTool.tslib/mcp/tools/research/registerResearchRankTool.tslib/mcp/tools/research/registerResearchSimilarTool.tslib/mcp/tools/research/registerResearchTrackTool.tslib/mcp/tools/research/registerResearchTracksTool.tslib/mcp/tools/research/registerResearchUrlsTool.tslib/mcp/tools/research/registerResearchVenuesTool.tslib/parallel/enrichEntity.tslib/parallel/extractUrl.tslib/research/getResearchChartsHandler.tslib/research/getResearchMilestonesHandler.tslib/research/getResearchRadioHandler.tslib/research/getResearchRankHandler.tslib/research/getResearchVenuesHandler.tslib/research/postResearchEnrichHandler.tslib/research/postResearchExtractHandler.ts
| const result = await proxyToChartmetric( | ||
| `/curator/${args.platform}/${args.id}`, | ||
| ); | ||
| return getToolResultSuccess(result.data); | ||
| } catch (error) { |
There was a problem hiding this comment.
Encode path params and handle non-2xx proxy results before returning success.
Line 29 interpolates raw user input into the path, and Line 31 returns success even when upstream returned an HTTP error payload.
Proposed fix
- const result = await proxyToChartmetric(
- `/curator/${args.platform}/${args.id}`,
- );
+ const platform = encodeURIComponent(args.platform);
+ const id = encodeURIComponent(args.id);
+ const result = await proxyToChartmetric(`/curator/${platform}/${id}`);
+ if (result.status >= 400) {
+ return getToolResultError(
+ (result.data as { error?: string })?.error ?? "Failed to fetch curator",
+ );
+ }
return getToolResultSuccess(result.data);Based on learnings: "Use getToolResultSuccess(data) to wrap successful MCP tool responses and getToolResultError(message) to wrap error responses".
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@lib/mcp/tools/research/registerResearchCuratorTool.ts` around lines 28 - 32,
The code should URL-encode user-supplied path params and return an error wrapper
for non-2xx upstream responses: encode args.platform and args.id with
encodeURIComponent when building the path passed to proxyToChartmetric (the call
in proxyToChartmetric(`/curator/${args.platform}/${args.id}`)), then inspect the
proxyToChartmetric response (e.g., result.status or result.ok) and if it is not
a success status return getToolResultError(...) with a clear message (use
result.data?.message or include result.status), otherwise return
getToolResultSuccess(result.data).
There was a problem hiding this comment.
Acknowledged — MCP curator tool should check proxy status before returning. Will address as follow-up for MCP tool error handling hardening.
There was a problem hiding this comment.
Rate Limit Exceeded
@sidneyswift have exceeded the limit for the number of chat messages per hour. Please wait 1 minutes and 37 seconds before sending another message.
Endpoint Test Results — Preview DeploymentPreview: Environment IssueAll Chartmetric-proxied GET endpoints return 500 because Results
Security Validation
Bug Fixes Verified (this PR)
Action RequiredAdd 🤖 Generated with Claude Code |
API Endpoint Testing ResultsTested all 30 endpoints in this PR against the preview deployment. Auth Rejection — ✅ All PassAll endpoints correctly return 401 without credentials. Validation Errors — ✅ All PassAll endpoints return proper 400 with descriptive error messages for missing/invalid params. GET Endpoints — Artist-scoped (15 endpoints)
GET Endpoints — Non-artist-scoped (6 endpoints)
GET Endpoints — Custom handlers (4 endpoints)
POST Endpoints (5 endpoints)
Issues Found
Summary: 27/30 endpoints passing ✅ | 2 upstream issues |
Docs vs Implementation ComparisonCompared the API docs (OpenAPI spec from docs PR #85) against live endpoint behavior on the preview deployment. 1.
|
llms.txt |
OpenAPI Spec | Implementation | |
|---|---|---|---|
| Search | POST /api/research/search |
GET /api/research |
GET /api/research |
The llms.txt file incorrectly lists the search endpoint as POST /api/research/search. The OpenAPI spec and implementation both use GET /api/research?q=.... This will confuse LLM consumers of the API.
2. /api/research/lookup — Response shape mismatch
Docs say: flat object with status, id, spotify_id, apple_music_id, deezer_id
Actual response: Chartmetric returns an array, and the handler spreads it into numbered keys:
{"0": {...}, "1": {...}, ..., "249": {...}, "status": "success"}The spread logic uses ...(typeof result.data === "object" && result.data !== null ? result.data : ...) but arrays are objects in JS, so ...arrayData creates {"0":...,"1":...}. Fix: add !Array.isArray(result.data) guard (like handleResearchRequest already does).
3. /api/research/enrich — Response key mismatch
| Field | Docs (OpenAPI) | Actual Response |
|---|---|---|
| Citations | research_basis.citations (nested) |
citations (top-level) |
Docs specify citations nested under research_basis.citations. Implementation returns citations at the top level alongside output and status.
4. /api/research/similar — Default path broken
Docs say artist is the only required param, with audience/genre/mood/musicality all optional. But the default Chartmetric path (/relatedartists) returns 400. Only the /by-configurations path works (requires at least one config param like genre=high).
Options:
- Make the docs state that at least one config param is required
- Or fix the fallback path in
getResearchSimilarHandler.ts
5. /api/research/charts — Upstream 400 with documented params
Docs say platform is the only required param. But GET /api/research/charts?platform=spotify returns 400 from Chartmetric. The upstream API likely needs additional params (e.g. type, interval, or date) that aren't documented as required.
6. Minor: Missing documented params in some endpoints
These params are documented in the OpenAPI spec but weren't tested since they're optional. Worth verifying they pass through correctly:
| Endpoint | Documented Optional Params |
|---|---|
/api/research/audience |
platform |
/api/research/playlists |
platform, status, editorial, since, sort, limit |
/api/research/discover |
country, genre, sp_monthly_listeners_min/max, sort, limit |
Summary
| Issue | Severity | Where to Fix |
|---|---|---|
llms.txt wrong method/path for search |
🔴 High | docs repo llms.txt |
/lookup array spread bug |
🔴 High | getResearchLookupHandler.ts |
/enrich citations path mismatch |
🟡 Medium | docs OpenAPI or handler |
/similar default path fails |
🟡 Medium | handler or docs |
/charts fails with documented params |
🟡 Medium | handler or docs |
Docs Fix Needed:
|
There was a problem hiding this comment.
8 issues found across 63 files (changes from recent commits).
Prompt for AI agents (unresolved issues)
Check if these issues are valid — if so, understand the root cause of each and fix them. If appropriate, use sub-agents to investigate and fix each issue separately.
<file name="app/api/research/metrics/route.ts">
<violation number="1" location="app/api/research/metrics/route.ts:17">
P2: Custom agent: **Flag AI Slop and Fabricated Changes**
These added JSDoc tags are narrating comments that restate the obvious code (`@param request - The incoming HTTP request`, `@returns The JSON response`). They add no useful context beyond what the type signature and one-line function body already convey. Remove them or replace with meaningful documentation (e.g., required query params, error responses, rate-limit behavior).</violation>
</file>
<file name="app/api/research/discover/route.ts">
<violation number="1" location="app/api/research/discover/route.ts:17">
P2: Custom agent: **Flag AI Slop and Fabricated Changes**
These added JSDoc tags (`@param request - The incoming HTTP request.`, `@returns The JSON response.`, `@returns A 200 response with CORS headers.`) restate what the types and one-line bodies already make obvious. They add noise without useful context, constraints, or intent — a pattern Rule 3 flags as narrating comments. Remove them or replace with genuinely informative descriptions (e.g., expected query params, error cases, auth requirements).</violation>
</file>
<file name="app/api/research/genres/route.ts">
<violation number="1" location="app/api/research/genres/route.ts:18">
P2: Custom agent: **Flag AI Slop and Fabricated Changes**
`@returns The JSON response.` and `@param request - The incoming HTTP request.` are narrating JSDoc tags that restate what's already obvious from the types and code. Either add comments that explain non-obvious behavior (e.g., auth requirements, error codes, rate limits) or remove the boilerplate tags.</violation>
</file>
<file name="app/api/research/web/route.ts">
<violation number="1" location="app/api/research/web/route.ts:19">
P2: Custom agent: **Flag AI Slop and Fabricated Changes**
All three added JSDoc lines are narrating comments that restate what is already obvious from the type signatures and one-line bodies. Rule 3 explicitly flags "narrating comments that mostly restate the next line or obvious code instead of adding useful context, constraints, or intent." Consider removing them or replacing with comments that describe non-obvious behavior (e.g., authentication requirements, rate limits, or error semantics).</violation>
</file>
<file name="app/api/research/profile/route.ts">
<violation number="1" location="app/api/research/profile/route.ts:17">
P2: Custom agent: **Flag AI Slop and Fabricated Changes**
These added JSDoc tags are narrating comments that restate what's already obvious from the types and one-line bodies (`@param request - The incoming HTTP request.`, `@returns The JSON response.`). They add no useful context about response shape, error cases, or constraints — classic low-value AI-generated filler. Remove them or replace with genuinely informative descriptions (e.g., expected query params, error codes, response shape).</violation>
</file>
<file name="app/api/research/similar/route.ts">
<violation number="1" location="app/api/research/similar/route.ts:17">
P2: Custom agent: **Flag AI Slop and Fabricated Changes**
These added JSDoc tags are narrating comments that restate what the types and code already make obvious (`@param request - The incoming HTTP request.` when typed `NextRequest`; `@returns The JSON response.` with no detail on shape or errors). Remove them or replace with genuinely useful info (e.g., supported query params, error status codes, response shape).</violation>
</file>
<file name="app/api/research/playlist/route.ts">
<violation number="1" location="app/api/research/playlist/route.ts:18">
P2: Custom agent: **Flag AI Slop and Fabricated Changes**
These added JSDoc tags are narrating comments that restate what the code and types already express. `@param request - The incoming HTTP request` adds nothing beyond the `NextRequest` type, and `@returns The JSON response` is completely vacuous — it says nothing about response shape, status codes, or error cases. Remove them or replace with genuinely useful information (e.g., expected query params, possible error responses).</violation>
</file>
<file name="app/api/research/instagram-posts/route.ts">
<violation number="1" location="app/api/research/instagram-posts/route.ts:18">
P2: Custom agent: **Flag AI Slop and Fabricated Changes**
These added JSDoc tags are narrating filler — `@param request - The incoming HTTP request.` restates the `NextRequest` type, and `@returns The JSON response.` says nothing about what the endpoint actually returns. Either remove them or replace with genuinely useful descriptions (e.g., the response shape or error conditions).</violation>
</file>
Reply with feedback, questions, or to request a fix. Tag @cubic-dev-ai to re-run a review, or fix all with cubic.
Full Test Results — 30/30 Endpoints + 5 Chained Research DemosTested on preview: All 30 Endpoints
Chain 1: Artist Intelligence BriefQuestion: "How is Kaash Paige doing?" Kaash Paige | Score: 85.5 | Label: Kaash Paige LLC Top cities:
AI insights:
Chain 2: Competitive LandscapeQuestion: "Who are Kaash Paige's competitors?"
Chain 3: Tour RoutingQuestion: "Where should Kaash Paige tour?" Top listener cities:
Recent venues: Milkboy, S.O.B.'s, The Cambridge Room at House Of Blues, Bronze Peacock Room Insight: Kaash Paige's top cities are international (Brazil, Indonesia, Thailand) but she's only played small US venues. Massive untapped international touring opportunity. Chain 4: Structured EnrichmentQuestion: "Get me structured facts about Kaash Paige" {
"full_name": "D'Kyla Paige Woolen",
"debut_year": 2018,
"hometown": "Dallas, Texas, United States",
"genre": "R&B",
"record_label": "Se Lavi Productions and Def Jam Recordings",
"biggest_song": "Love Songs"
}6 sources cited Chain 5: Industry ContactsQuestion: "Find people connected to Kaash Paige" |
There was a problem hiding this comment.
3 issues found across 44 files (changes from recent commits).
Prompt for AI agents (unresolved issues)
Check if these issues are valid — if so, understand the root cause of each and fix them. If appropriate, use sub-agents to investigate and fix each issue separately.
<file name="lib/research/proxyToChartmetric.ts">
<violation number="1" location="lib/research/proxyToChartmetric.ts:54">
P1: Custom agent: **Flag AI Slop and Fabricated Changes**
Silent, unlogged `catch` returning `{ data: null, status: 500 }` is a low-quality AI-generated safety pattern. It swallows all errors (network failures, token issues, JSON parse errors) with no logging and no error details, making production debugging impossible across all 20 endpoints that depend on this function. The new catch path also has no test coverage despite being trivially testable.</violation>
</file>
<file name="lib/research/getResearchDiscoverHandler.ts">
<violation number="1" location="lib/research/getResearchDiscoverHandler.ts:27">
P2: The `sp_ml[]` array parameter is sent as a single comma-separated value (`sp_ml[]=100,1000`) instead of repeated keys (`sp_ml[]=100&sp_ml[]=1000`). The `proxyToChartmetric` helper uses `URLSearchParams.set()` on a `Record<string, string>`, which fundamentally can't represent multi-value keys. If Chartmetric expects standard array-param encoding, this filter will silently produce wrong results when both min and max are provided. Consider extending `queryParams` to accept `Record<string, string | string[]>` and using `url.searchParams.append()` for array values.</violation>
</file>
<file name="lib/exa/searchPeople.ts">
<violation number="1" location="lib/exa/searchPeople.ts:31">
P2: `Math.min`/`Math.max` don't guard against `NaN` — all three calls propagate it unchanged. If a caller passes `NaN` (e.g. from a failed parse), the Exa API receives `numResults: NaN`. Add a `NaN` fallback to the default.</violation>
</file>
Reply with feedback, questions, or to request a fix. Tag @cubic-dev-ai to re-run a review, or fix all with cubic.
| const mlMin = sp.get("sp_monthly_listeners_min"); | ||
| const mlMax = sp.get("sp_monthly_listeners_max"); | ||
| if (mlMin && mlMax) { | ||
| params["sp_ml[]"] = `${mlMin},${mlMax}`; |
There was a problem hiding this comment.
P2: The sp_ml[] array parameter is sent as a single comma-separated value (sp_ml[]=100,1000) instead of repeated keys (sp_ml[]=100&sp_ml[]=1000). The proxyToChartmetric helper uses URLSearchParams.set() on a Record<string, string>, which fundamentally can't represent multi-value keys. If Chartmetric expects standard array-param encoding, this filter will silently produce wrong results when both min and max are provided. Consider extending queryParams to accept Record<string, string | string[]> and using url.searchParams.append() for array values.
Prompt for AI agents
Check if this issue is valid — if so, understand the root cause and fix it. At lib/research/getResearchDiscoverHandler.ts, line 27:
<comment>The `sp_ml[]` array parameter is sent as a single comma-separated value (`sp_ml[]=100,1000`) instead of repeated keys (`sp_ml[]=100&sp_ml[]=1000`). The `proxyToChartmetric` helper uses `URLSearchParams.set()` on a `Record<string, string>`, which fundamentally can't represent multi-value keys. If Chartmetric expects standard array-param encoding, this filter will silently produce wrong results when both min and max are provided. Consider extending `queryParams` to accept `Record<string, string | string[]>` and using `url.searchParams.append()` for array values.</comment>
<file context>
@@ -23,10 +21,15 @@ export async function getResearchDiscoverHandler(request: NextRequest) {
+ const mlMin = sp.get("sp_monthly_listeners_min");
+ const mlMax = sp.get("sp_monthly_listeners_max");
+ if (mlMin && mlMax) {
+ params["sp_ml[]"] = `${mlMin},${mlMax}`;
+ } else if (mlMin) {
+ params["sp_ml[]"] = mlMin;
</file context>
There was a problem hiding this comment.
Fixed — when both mlMin and mlMax are provided, combined as comma-separated value in a single param.
There was a problem hiding this comment.
Thanks for the update. If comma-separated is the expected encoding for Chartmetric here, I’ll treat this as intended; otherwise we can switch to repeated keys.
| query: string, | ||
| numResults: number = 10, | ||
| ): Promise<ExaPeopleResponse> { | ||
| const safeNumResults = Math.min(100, Math.max(1, Math.floor(numResults))); |
There was a problem hiding this comment.
P2: Math.min/Math.max don't guard against NaN — all three calls propagate it unchanged. If a caller passes NaN (e.g. from a failed parse), the Exa API receives numResults: NaN. Add a NaN fallback to the default.
Prompt for AI agents
Check if this issue is valid — if so, understand the root cause and fix it. At lib/exa/searchPeople.ts, line 31:
<comment>`Math.min`/`Math.max` don't guard against `NaN` — all three calls propagate it unchanged. If a caller passes `NaN` (e.g. from a failed parse), the Exa API receives `numResults: NaN`. Add a `NaN` fallback to the default.</comment>
<file context>
@@ -28,6 +28,7 @@ export async function searchPeople(
query: string,
numResults: number = 10,
): Promise<ExaPeopleResponse> {
+ const safeNumResults = Math.min(100, Math.max(1, Math.floor(numResults)));
const apiKey = process.env.EXA_API_KEY;
</file context>
| const safeNumResults = Math.min(100, Math.max(1, Math.floor(numResults))); | |
| const floored = Math.floor(numResults); | |
| const safeNumResults = Number.isNaN(floored) ? 10 : Math.min(100, Math.max(1, floored)); |
There was a problem hiding this comment.
Fixed — numResults is clamped with Math.min(100, Math.max(1, Math.floor(numResults))) at the top of searchPeople().
There was a problem hiding this comment.
Thanks for the update! Note that Math.min/Max still propagate NaN—consider a Number.isNaN fallback (e.g., default to 10) to fully guard against NaN.
Final Verification — 30/30 Endpoints PassingTested on preview after all fixes: Every endpoint was tested against the docs schemas in docs#101. Response keys compared to OpenAPI component schema fields.
Docs coverageAll 30 endpoints in docs#101 have:
Tests
|
There was a problem hiding this comment.
15 issues found across 162 files (changes from recent commits).
Note: This PR contains a large number of files. cubic only reviews up to 75 files per PR, so some files may not have been reviewed.
Prompt for AI agents (unresolved issues)
Check if these issues are valid — if so, understand the root cause of each and fix them. If appropriate, use sub-agents to investigate and fix each issue separately.
<file name="app/api/chats/[id]/messages/route.ts">
<violation number="1" location="app/api/chats/[id]/messages/route.ts:11">
P1: Custom agent: **Module should export a single primary function whose name matches the filename**
This module violates the “single primary function per module” rule by exporting both `OPTIONS` and `GET` in the same file.</violation>
</file>
<file name="app/api/chats/[id]/messages/trailing/route.ts">
<violation number="1" location="app/api/chats/[id]/messages/trailing/route.ts:9">
P1: Custom agent: **Module should export a single primary function whose name matches the filename**
Violates Rule 1 (single primary export): `route.ts` exports multiple top-level functions (`OPTIONS` and `DELETE`) instead of one primary function matching the filename.</violation>
</file>
<file name="app/api/chats/[id]/segment/route.ts">
<violation number="1" location="app/api/chats/[id]/segment/route.ts:11">
P1: Custom agent: **Module should export a single primary function whose name matches the filename**
Rule 1 violation: this module exports multiple top-level functions (`OPTIONS` and `GET`) instead of a single primary export matching the filename (`route`).</violation>
</file>
<file name="app/api/chats/[id]/artist/route.ts">
<violation number="1" location="app/api/chats/[id]/artist/route.ts:11">
P1: Custom agent: **Module should export a single primary function whose name matches the filename**
Rule 1 violation: this module exports multiple top-level functions (`OPTIONS` and `GET`) instead of a single primary export matching `route.ts`.</violation>
</file>
<file name="app/api/chats/[id]/messages/copy/route.ts">
<violation number="1" location="app/api/chats/[id]/messages/copy/route.ts:11">
P1: Custom agent: **Module should export a single primary function whose name matches the filename**
Rule 1 violation: this module exports multiple top-level functions (`OPTIONS` and `POST`) instead of a single primary export matching the filename.</violation>
</file>
<file name="lib/chats/validateChatAccess.ts">
<violation number="1" location="lib/chats/validateChatAccess.ts:52">
P3: `if (!params)` is unreachable here, so this branch is dead code and adds unnecessary complexity.</violation>
</file>
<file name="lib/chats/validateCopyChatMessagesBody.ts">
<violation number="1" location="lib/chats/validateCopyChatMessagesBody.ts:52">
P2: Self-copy guard is case-sensitive, so the same UUID with different letter casing can bypass the check.</violation>
</file>
<file name="lib/agents/content/downloadVideoBuffer.ts">
<violation number="1" location="lib/agents/content/downloadVideoBuffer.ts:9">
P2: Add a timeout to the video download fetch so one stalled URL does not block the entire result batch.</violation>
<violation number="2" location="lib/agents/content/downloadVideoBuffer.ts:12">
P2: Avoid logging the full video URL in error messages, because query tokens in signed URLs can be exposed in logs.</violation>
</file>
<file name="lib/agents/content/handlers/registerOnNewMention.ts">
<violation number="1" location="lib/agents/content/handlers/registerOnNewMention.ts:30">
P2: Delay attachment extraction until after artist/repo validation to avoid uploading public media for requests that fail early.</violation>
</file>
<file name="lib/chats/copyChatMessagesHandler.ts">
<violation number="1" location="lib/chats/copyChatMessagesHandler.ts:51">
P1: `clearExisting` performs delete-before-insert without atomicity, so an insert failure can permanently wipe the target chat messages.</violation>
</file>
<file name="lib/chats/validateDeleteTrailingMessagesQuery.ts">
<violation number="1" location="lib/chats/validateDeleteTrailingMessagesQuery.ts:50">
P2: Handle `selectMemories` returning `null` before the not-found check; otherwise database failures are returned as 404 "Message not found" instead of an internal error.</violation>
</file>
<file name="lib/chats/getChatMessagesHandler.ts">
<violation number="1" location="lib/chats/getChatMessagesHandler.ts:34">
P2: Return a flat, named success payload instead of a `data` wrapper to keep this endpoint consistent with the project’s API response contract.</violation>
</file>
<file name="lib/agents/content/resolveAttachmentUrl.ts">
<violation number="1" location="lib/agents/content/resolveAttachmentUrl.ts:41">
P1: Handle `Blob` values explicitly before calling `Buffer.from` to avoid runtime errors when `attachment.data` is a Blob.</violation>
</file>
<file name="lib/chats/getChatSegmentHandler.ts">
<violation number="1" location="lib/chats/getChatSegmentHandler.ts:25">
P2: Wrap the segment lookup in a try/catch so Supabase errors return a controlled 500 response instead of an unhandled exception.</violation>
</file>
Reply with feedback, questions, or to request a fix. Tag @cubic-dev-ai to re-run a review, or fix all with cubic.
| * | ||
| * @returns Empty response with CORS headers. | ||
| */ | ||
| export async function OPTIONS(): Promise<NextResponse> { |
There was a problem hiding this comment.
P1: Custom agent: Module should export a single primary function whose name matches the filename
This module violates the “single primary function per module” rule by exporting both OPTIONS and GET in the same file.
Prompt for AI agents
Check if this issue is valid — if so, understand the root cause and fix it. At app/api/chats/[id]/messages/route.ts, line 11:
<comment>This module violates the “single primary function per module” rule by exporting both `OPTIONS` and `GET` in the same file.</comment>
<file context>
@@ -0,0 +1,35 @@
+ *
+ * @returns Empty response with CORS headers.
+ */
+export async function OPTIONS(): Promise<NextResponse> {
+ return new NextResponse(null, {
+ status: 200,
</file context>
| /** | ||
| * OPTIONS handler for CORS preflight requests. | ||
| */ | ||
| export async function OPTIONS() { |
There was a problem hiding this comment.
P1: Custom agent: Module should export a single primary function whose name matches the filename
Violates Rule 1 (single primary export): route.ts exports multiple top-level functions (OPTIONS and DELETE) instead of one primary function matching the filename.
Prompt for AI agents
Check if this issue is valid — if so, understand the root cause and fix it. At app/api/chats/[id]/messages/trailing/route.ts, line 9:
<comment>Violates Rule 1 (single primary export): `route.ts` exports multiple top-level functions (`OPTIONS` and `DELETE`) instead of one primary function matching the filename.</comment>
<file context>
@@ -0,0 +1,27 @@
+/**
+ * OPTIONS handler for CORS preflight requests.
+ */
+export async function OPTIONS() {
+ return new NextResponse(null, {
+ status: 200,
</file context>
| * | ||
| * @returns A NextResponse with CORS headers. | ||
| */ | ||
| export async function OPTIONS() { |
There was a problem hiding this comment.
P1: Custom agent: Module should export a single primary function whose name matches the filename
Rule 1 violation: this module exports multiple top-level functions (OPTIONS and GET) instead of a single primary export matching the filename (route).
Prompt for AI agents
Check if this issue is valid — if so, understand the root cause and fix it. At app/api/chats/[id]/segment/route.ts, line 11:
<comment>Rule 1 violation: this module exports multiple top-level functions (`OPTIONS` and `GET`) instead of a single primary export matching the filename (`route`).</comment>
<file context>
@@ -0,0 +1,35 @@
+ *
+ * @returns A NextResponse with CORS headers.
+ */
+export async function OPTIONS() {
+ return new NextResponse(null, {
+ status: 200,
</file context>
| * | ||
| * @returns A NextResponse with CORS headers. | ||
| */ | ||
| export async function OPTIONS() { |
There was a problem hiding this comment.
P1: Custom agent: Module should export a single primary function whose name matches the filename
Rule 1 violation: this module exports multiple top-level functions (OPTIONS and GET) instead of a single primary export matching route.ts.
Prompt for AI agents
Check if this issue is valid — if so, understand the root cause and fix it. At app/api/chats/[id]/artist/route.ts, line 11:
<comment>Rule 1 violation: this module exports multiple top-level functions (`OPTIONS` and `GET`) instead of a single primary export matching `route.ts`.</comment>
<file context>
@@ -0,0 +1,35 @@
+ *
+ * @returns A NextResponse with CORS headers.
+ */
+export async function OPTIONS() {
+ return new NextResponse(null, {
+ status: 200,
</file context>
| * | ||
| * @returns A NextResponse with CORS headers. | ||
| */ | ||
| export async function OPTIONS() { |
There was a problem hiding this comment.
P1: Custom agent: Module should export a single primary function whose name matches the filename
Rule 1 violation: this module exports multiple top-level functions (OPTIONS and POST) instead of a single primary export matching the filename.
Prompt for AI agents
Check if this issue is valid — if so, understand the root cause and fix it. At app/api/chats/[id]/messages/copy/route.ts, line 11:
<comment>Rule 1 violation: this module exports multiple top-level functions (`OPTIONS` and `POST`) instead of a single primary export matching the filename.</comment>
<file context>
@@ -0,0 +1,34 @@
+ *
+ * @returns A NextResponse with CORS headers.
+ */
+export async function OPTIONS() {
+ return new NextResponse(null, {
+ status: 200,
</file context>
| ); | ||
|
|
||
| // Extract audio/image attachments from the Slack message | ||
| const { songUrl, imageUrl } = await extractMessageAttachments(message); |
There was a problem hiding this comment.
P2: Delay attachment extraction until after artist/repo validation to avoid uploading public media for requests that fail early.
Prompt for AI agents
Check if this issue is valid — if so, understand the root cause and fix it. At lib/agents/content/handlers/registerOnNewMention.ts, line 30:
<comment>Delay attachment extraction until after artist/repo validation to avoid uploading public media for requests that fail early.</comment>
<file context>
@@ -4,23 +4,30 @@ import { triggerPollContentRun } from "@/lib/trigger/triggerPollContentRun";
+ );
+
+ // Extract audio/image attachments from the Slack message
+ const { songUrl, imageUrl } = await extractMessageAttachments(message);
// Resolve artist slug
</file context>
| ); | ||
| } | ||
|
|
||
| const [memory] = |
There was a problem hiding this comment.
P2: Handle selectMemories returning null before the not-found check; otherwise database failures are returned as 404 "Message not found" instead of an internal error.
Prompt for AI agents
Check if this issue is valid — if so, understand the root cause and fix it. At lib/chats/validateDeleteTrailingMessagesQuery.ts, line 50:
<comment>Handle `selectMemories` returning `null` before the not-found check; otherwise database failures are returned as 404 "Message not found" instead of an internal error.</comment>
<file context>
@@ -0,0 +1,67 @@
+ );
+ }
+
+ const [memory] =
+ (await selectMemories(roomResult.room.id, {
+ memoryId: parsedQuery.data.from_message_id,
</file context>
| ); | ||
| } | ||
|
|
||
| return NextResponse.json({ data: memories }, { status: 200, headers: getCorsHeaders() }); |
There was a problem hiding this comment.
P2: Return a flat, named success payload instead of a data wrapper to keep this endpoint consistent with the project’s API response contract.
Prompt for AI agents
Check if this issue is valid — if so, understand the root cause and fix it. At lib/chats/getChatMessagesHandler.ts, line 34:
<comment>Return a flat, named success payload instead of a `data` wrapper to keep this endpoint consistent with the project’s API response contract.</comment>
<file context>
@@ -0,0 +1,42 @@
+ );
+ }
+
+ return NextResponse.json({ data: memories }, { status: 200, headers: getCorsHeaders() });
+ } catch (error) {
+ console.error("Unexpected error in getChatMessagesHandler:", error);
</file context>
| return roomResult; | ||
| } | ||
|
|
||
| const segmentRoom = await selectSegmentRoomByRoomId(roomResult.room.id); |
There was a problem hiding this comment.
P2: Wrap the segment lookup in a try/catch so Supabase errors return a controlled 500 response instead of an unhandled exception.
Prompt for AI agents
Check if this issue is valid — if so, understand the root cause and fix it. At lib/chats/getChatSegmentHandler.ts, line 25:
<comment>Wrap the segment lookup in a try/catch so Supabase errors return a controlled 500 response instead of an unhandled exception.</comment>
<file context>
@@ -0,0 +1,36 @@
+ return roomResult;
+ }
+
+ const segmentRoom = await selectSegmentRoomByRoomId(roomResult.room.id);
+
+ return NextResponse.json(
</file context>
| ); | ||
| } | ||
|
|
||
| const { params, error } = await buildGetChatsParams({ |
There was a problem hiding this comment.
P3: if (!params) is unreachable here, so this branch is dead code and adds unnecessary complexity.
Prompt for AI agents
Check if this issue is valid — if so, understand the root cause and fix it. At lib/chats/validateChatAccess.ts, line 52:
<comment>`if (!params)` is unreachable here, so this branch is dead code and adds unnecessary complexity.</comment>
<file context>
@@ -0,0 +1,71 @@
+ );
+ }
+
+ const { params, error } = await buildGetChatsParams({
+ account_id: accountId,
+ });
</file context>
|
You're iterating quickly on this pull request. To help protect your rate limits, cubic has paused automatic reviews on new pushes for now—when you're ready for another review, comment |
…d tests Research primitive — provider-agnostic music industry research: - 30 REST endpoints under /api/research/ (Chartmetric, Perplexity, Exa, Parallel) - 28 MCP tools with proper auth (resolveAccountId) and credit deduction - 2 shared handlers (handleArtistResearch, handleResearchRequest) for DRY - Zod validation on discover endpoint - 10 test files (token, proxy, artist resolution, charts, lookup, similar, search, track, web, discover) - Source param allowlist on metrics to prevent path injection - proxyToChartmetric wrapped in try/catch for consistent error contract - All 1767 tests passing, 0 lint errors in research files Made-with: Cursor
65a78d0 to
6d1f805
Compare
Track-level playlist lookup — returns editorial, indie, and algorithmic
playlists for a specific track. Accepts Chartmetric track ID or track
name (resolved via search). Proxies to Chartmetric
/track/{id}/{platform}/{status}/playlists.
Includes route handler, domain handler, MCP tool, and 8 unit tests.
Made-with: Cursor
Adds resolveTrack() — searches Spotify first (accurate matching with artist: filter), maps Spotify track ID to Chartmetric ID, falls back to Chartmetric search if Spotify fails. Adds optional artist= param to track/playlists endpoint and MCP tool. Made-with: Cursor
Spotify search returns ISRC, which maps to Chartmetric more reliably
than Spotify track ID. Tries /track/isrc/{isrc} first, then
/track/spotify/{id}, then falls back to Chartmetric text search.
Made-with: Cursor
Spotify search finds exact track name, then we match against the artist's Chartmetric playlists/tracks by name to get the cm_track ID. Avoids Chartmetric's broken text search and unreliable ID mapping. Made-with: Cursor
Maps ISRC → chartmetric_ids via the correct endpoint path. Falls back to Spotify track ID if ISRC lookup fails. Platform-agnostic. Made-with: Cursor
Made-with: Cursor
Summary
/api/research/backed by Chartmetric, matching the Recoup API query-param patternobjwrapper stripped from responses?artist=Drakeresolves internally, no provider IDs exposedproxyToChartmetric,resolveArtist,handleArtistResearchEndpoints
Search, Lookup, Profile, Metrics (14 platforms), Audience, Cities, Similar, URLs, Instagram Posts, Playlists, Albums, Tracks, Career, Insights, Track, Playlist, Curator, Discover, Genres, Festivals
Deferred
POST /api/research/web) and Deep research (POST /api/research/deep) — separate PRobjstripping (field renames per OpenAPI schema)Docs spec
recoupable/docs#85
Test plan
CHARTMETRIC_REFRESH_TOKENto Vercel env and test live endpointsMade with Cursor
Summary by CodeRabbit